Andrés Delfino Reporter
	Description • 17 years ago

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3pre) Gecko/2008020413 Minefield/3.0b3pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3pre) Gecko/2008020413 Minefield/3.0b3pre

Firefox main window menus has icons that can (and should) be used in Source viewer:

File ▸ Save Page As...
File ▸ Print Preview
File ▸ Print...
File ▸ Close

Edit ▸ Undo
Edit ▸ Redo
Edit ▸ Cut
Edit ▸ Copy
Edit ▸ Paste
Edit ▸ Delete
Edit ▸ Select All
Edit ▸ Find
Edit ▸ Find Again

View ▸ Reload

Help ▸ Help Contents
Help ▸ About Minefield

Reproducible: Always

	Andrés Delfino Reporter
	Comment 1 • 17 years ago

Also include context menu items.

	Mike Beltzner [:beltzner, not reading bugmail]
	Comment 2 • 17 years ago

This does not block the final release of Firefox 3.

	Ian Spence Assignee
	Comment 4 • 17 years ago

I'm not even sure this is possible without some ugly hacking.  The stylesheet for view source is not part of the standard theme hierarchy, so I don't think it can be styled.

The other option would be throwing all that style stuff into the only other file included by the source file, and that's global.css

The other, best theoretical option is to just add a stylesheet to each of the themes called viewSource.css that could contain this, but I wonder if there is a reason this is the only dialog not styled by themes

	Andrés Delfino Reporter
	Comment 5 • 17 years ago

IMHO, having a viewSource.css would be the best solution, but I don't know how hard would it be to change this.

I guess themes don't care about the View source window, but system integration does :P

	Ian Spence Assignee
	Comment 6 • 17 years ago

This is my first attempt at a fix.  I wasn't sure where the file should go, so I threw it into toolkit/themes/[theme]/mozapps/viewsource (a directory I created)

I didn't move the styling for the syntax highlighting in there since I don't think we need to make that part of every theme, but I made sure that I include this file in the page after the syntax highlighting file so that themes, if they wnated to, could override the highlighting

	Reed Loden [:reed]
	Comment 7 • 17 years ago

Comment on attachment 309633 [details] [diff] [review]
Add menu icons to the View Source dialog

>+        skin/classic/aero/viewsource/viewsource.css                        (viewsource/viewsource.css)

This is missing a "mozapps/" in the path.

	Ian Spence Assignee
	Comment 8 • 17 years ago

Alas, Roc has thwarted my plan to destroy Vista from the inside.  I've updated my patch accordingly

	Asaf Romano (gone)
	Comment 9 • 17 years ago

Comment on attachment 309680 [details] [diff] [review]
Updated to support Vista

Please add ids to styled menu-items instead. The non-stub style file should include a license header.

	Ian Spence Assignee
	Comment 10 • 17 years ago

Uses ids instead of cmd attributes and adds a license header to the gnomestripe css file

	Asaf Romano (gone)
	Comment 11 • 17 years ago

Comment on attachment 309705 [details] [diff] [review]
Addresses comments

r=mano

	Mike Beltzner [:beltzner, not reading bugmail]
	Comment 12 • 17 years ago

Comment on attachment 309705 [details] [diff] [review]
Addresses comments

a1.9=beltzner

	Reed Loden [:reed]
	Comment 13 • 17 years ago

Checking in toolkit/components/viewsource/content/viewPartialSource.xul;
/cvsroot/mozilla/toolkit/components/viewsource/content/viewPartialSource.xul,v  <--  viewPartialSource.xul
new revision: 1.29; previous revision: 1.28
done
Checking in toolkit/components/viewsource/content/viewSource.xul;
/cvsroot/mozilla/toolkit/components/viewsource/content/viewSource.xul,v  <--  viewSource.xul
new revision: 1.37; previous revision: 1.36
done
Checking in toolkit/themes/gnomestripe/mozapps/jar.mn;
/cvsroot/mozilla/toolkit/themes/gnomestripe/mozapps/jar.mn,v  <--  jar.mn
new revision: 1.11; previous revision: 1.10
done
RCS file: /cvsroot/mozilla/toolkit/themes/gnomestripe/mozapps/viewsource/viewsource.css,v
done
Checking in toolkit/themes/gnomestripe/mozapps/viewsource/viewsource.css;
/cvsroot/mozilla/toolkit/themes/gnomestripe/mozapps/viewsource/viewsource.css,v  <--  viewsource.css
initial revision: 1.1
done
Checking in toolkit/themes/pinstripe/mozapps/jar.mn;
/cvsroot/mozilla/toolkit/themes/pinstripe/mozapps/jar.mn,v  <--  jar.mn
new revision: 1.27; previous revision: 1.26
done
RCS file: /cvsroot/mozilla/toolkit/themes/pinstripe/mozapps/viewsource/viewsource.css,v
done
Checking in toolkit/themes/pinstripe/mozapps/viewsource/viewsource.css;
/cvsroot/mozilla/toolkit/themes/pinstripe/mozapps/viewsource/viewsource.css,v  <--  viewsource.css
initial revision: 1.1
done
Checking in toolkit/themes/winstripe/mozapps/jar.mn;
/cvsroot/mozilla/toolkit/themes/winstripe/mozapps/jar.mn,v  <--  jar.mn
new revision: 1.32; previous revision: 1.31
done
RCS file: /cvsroot/mozilla/toolkit/themes/winstripe/mozapps/viewsource/viewsource.css,v
done
Checking in toolkit/themes/winstripe/mozapps/viewsource/viewsource.css;
/cvsroot/mozilla/toolkit/themes/winstripe/mozapps/viewsource/viewsource.css,v  <--  viewsource.css
initial revision: 1.1
done

	Hideo Oshima
	Comment 14 • 17 years ago

Help -> Help Contents in main menu of browser has an icon
but one of source viewer doesn't have.
Is this missed?

	Andrés Delfino Reporter
	Comment 15 • 17 years ago

Reopening per comment 14.

	Ian Spence Assignee
	Comment 17 • 17 years ago

Quick fix that adds an id to the Help Contents menuitem

	Asaf Romano (gone)
	Comment 18 • 17 years ago

Comment on attachment 310754 [details] [diff] [review]
Adds an id to the Help Contents menuitem

r=mano

	Reed Loden [:reed]
	Comment 19 • 17 years ago

Comment on attachment 310754 [details] [diff] [review]
Adds an id to the Help Contents menuitem

Literally just adds an id to a menuitem so the menu icon shows up.

	Mike Beltzner [:beltzner, not reading bugmail]
	Comment 20 • 17 years ago

Comment on attachment 310754 [details] [diff] [review]
Adds an id to the Help Contents menuitem

a=beltzner, whatever

	Reed Loden [:reed]
	Comment 21 • 17 years ago

Checking in browser/base/content/baseMenuOverlay.xul;
/cvsroot/mozilla/browser/base/content/baseMenuOverlay.xul,v  <--  baseMenuOverlay.xul
new revision: 1.18; previous revision: 1.17
done

	Reed Loden [:reed]
	Comment 22 • 17 years ago

Attachment 310754 [details] [diff] was backed out by bug 423486, so it needs to be relanded.

	Ian Spence Assignee
	Comment 23 • 17 years ago

The patch likely won't apply due to the bitrot caused by that bug.

	Reed Loden [:reed]
	Comment 24 • 17 years ago

(In reply to comment #23)
> The patch likely won't apply due to the bitrot caused by that bug.

Not a problem. I can manually fix it. :)

	Reed Loden [:reed]
	Comment 25 • 17 years ago

Checking in browser/base/content/baseMenuOverlay.xul;
/cvsroot/mozilla/browser/base/content/baseMenuOverlay.xul,v  <--  baseMenuOverlay.xul
new revision: 1.20; previous revision: 1.19
done

	neil@parkwaycc.co.uk
	Comment 26 • 16 years ago

(In reply to comment #6)
> This is my first attempt at a fix.  I wasn't sure where the file should go, so
> I threw it into toolkit/themes/[theme]/mozapps/viewsource (a directory I
> created)
IMHO chrome://global/content/view*.xul should use global themes only...

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 27 • 16 years ago

(In reply to comment #26)
> IMHO chrome://global/content/view*.xul should use global themes only...

File a bug on moving it?