Closed Bug 415664 Opened 17 years ago Closed 17 years ago

Source viewer needs menu icons

Categories

(Firefox :: Theme, defect)

x86
Linux
defect
Not set
trivial

Tracking

()

VERIFIED FIXED
Firefox 3 beta5

People

(Reporter: adelfino, Assigned: ispence)

References

Details

Attachments

(3 files, 2 obsolete files)

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
Version: unspecified → Trunk
Flags: blocking-firefox3?
Also include context menu items.
This does not block the final release of Firefox 3.
Status: UNCONFIRMED → NEW
Component: View Source → Theme
Ever confirmed: true
Flags: wanted-firefox3+
Flags: blocking-firefox3?
Flags: blocking-firefox3-
QA Contact: view.source → theme
Blocks: 405165
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
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
Attached patch Add menu icons to the View Source dialog (obsolete) (deleted) — Splinter Review
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
Assignee: nobody → ispence
Status: NEW → ASSIGNED
Attachment #309633 - Flags: review?(mano)
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.
Attached patch Updated to support Vista (obsolete) (deleted) — Splinter Review
Alas, Roc has thwarted my plan to destroy Vista from the inside. I've updated my patch accordingly
Attachment #309633 - Attachment is obsolete: true
Attachment #309680 - Flags: review?(mano)
Attachment #309633 - Flags: review?(mano)
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.
Attachment #309680 - Flags: review?(mano) → review-
Attached patch Addresses comments (deleted) — Splinter Review
Uses ids instead of cmd attributes and adds a license header to the gnomestripe css file
Attachment #309680 - Attachment is obsolete: true
Attachment #309705 - Flags: review?(mano)
Comment on attachment 309705 [details] [diff] [review] Addresses comments r=mano
Attachment #309705 - Flags: review?(mano) → review+
Attachment #309705 - Flags: approval1.9?
Comment on attachment 309705 [details] [diff] [review] Addresses comments a1.9=beltzner
Attachment #309705 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
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
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 beta5
Help -> Help Contents in main menu of browser has an icon but one of source viewer doesn't have. Is this missed?
Reopening per comment 14.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Quick fix that adds an id to the Help Contents menuitem
Attachment #310754 - Flags: review?(mano)
Comment on attachment 310754 [details] [diff] [review] Adds an id to the Help Contents menuitem r=mano
Attachment #310754 - Flags: review?(mano)
Attachment #310754 - Flags: review+
Attachment #310754 - Flags: approval1.9?
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.
Attachment #310754 - Flags: approval1.9b5?
Comment on attachment 310754 [details] [diff] [review] Adds an id to the Help Contents menuitem a=beltzner, whatever
Attachment #310754 - Flags: approval1.9b5?
Attachment #310754 - Flags: approval1.9b5+
Attachment #310754 - Flags: approval1.9?
Attachment #310754 - Flags: approval1.9+
Keywords: checkin-needed
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
Status: REOPENED → RESOLVED
Closed: 17 years ago17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Blocks: 381206
Status: RESOLVED → VERIFIED
Attachment 310754 [details] [diff] was backed out by bug 423486, so it needs to be relanded.
Keywords: checkin-needed
The patch likely won't apply due to the bitrot caused by that bug.
(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. :)
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
Keywords: checkin-needed
(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...
(In reply to comment #26) > IMHO chrome://global/content/view*.xul should use global themes only... File a bug on moving it?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: