Closed
Bug 268776
Opened 20 years ago
Closed 19 years ago
Help Viewer uses browser chrome
Categories
(SeaMonkey :: Help Viewer, defect)
SeaMonkey
Help Viewer
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.8beta4
People
(Reporter: Bugzilla-alanjstrBugs, Assigned: jwalden+fxhelp)
References
()
Details
Attachments
(2 files, 3 obsolete files)
43.92 KB,
application/zip
|
Details | |
14.07 KB,
patch
|
benjamin
:
approval1.8b4+
|
Details | Diff | Splinter Review |
Some of the URIs point to chrome://help/skin/ and some to chrome://browser/skin/ This is inconsistent and forces themes to skin the Help window. The Help Window should be self contained in terms of styles. Unfortunately this was not noticed until 1.0 was released, so now I have to re-skin. Even if the build script has to copy the images (although it appears in help.jar that it already does), these should be chrome://help/skin. for example: 41 .toolbarbutton-1 { 42 -moz-box-orient: vertical; 43 min-width: 0px; 44 list-style-image: url("chrome://help/skin/Toolbar.png"); 45 } 46 47 .browserButton { 48 -moz-box-orient: vertical; 49 min-width: 0px; 50 list-style-image: url("chrome://browser/skin/Toolbar.png"); 51 }
Assignee | ||
Comment 1•20 years ago
|
||
Okay, so what exactly do you want here? There's an obvious toolkit dependency on browser here which will be eradicated in bug 260058; how exactly it will be eradicated is still uncertain. Because Help uses many of the images the browser uses, I was thinking of creating a default skin for Help (with paths to images in chrome://help/skin/) but overriding it when building Firefox to reduce the download size by overriding the toolkit help.css with a browser-specific help.css (with paths to the relevant images in chrome://browser/skin/) and somehow removing the images from the help.jar package. I hadn't ironed out the details of exactly how this would work yet. If you want only chrome://help/skin/ images, that's easy enough to do. As I was planning on using as many chrome://browser/skin/ as possible to reduce build size, tho, could you explain exactly what benefits are gained by using only chrome://help/skin/ images? It's unclear what the pros of this are, while the cons (bigger build size, more duplication, more effort needed to sync button images) are pretty obvious to me.
Blocks: 260058
There are two approaches: 1) Use browser's theme's for everything and not have your own. That means have it use the same CSS as browser.css, but not literally. Maybe an @import if you have to. If I am going to theme the Help Viewer, I'd want it to be the same as my browser theming, and of course I don't want to maintain the same code in many places. Pinball is already riddled with that. 2) Be self-contained. Any changes to the browser's chrome will not affect you. If a theme does not include Help Viewer css, then they will see the defaults. The downsides to this include having to change your CSS every time they alter one of the standard buttons. Obviously I prefer option 1. Why have #help-back-button and not just use #back-button ? Any images you have that aren't in classic.jar will need to be moved, of course.
There's also inconsistent use of .toolbarbutton-1 and .browserButton etc etc
Comment 4•20 years ago
|
||
Jeff, you might want to talk to ben about this first. Help originally didn't use browser buttons but he requested that it be done in order to reduce bloat. And if ben doesn't like it than it won't go in :).
Comment 5•20 years ago
|
||
Maybe we need/want to have certain "universal" buttons as part of the core theme in toolkit? This isn't super-critical because there's no overlap because we have multiple themes in /toolkit that aren't currently shared, but if we have a Winstripe for Tbird replacing Qute at some point, it makes sense to have the icons in a single place. Or, alternatively, move the Help css into the app directory so each app skins stuff their own way.
Assignee | ||
Comment 6•20 years ago
|
||
(In reply to comment #5) > Maybe we need/want to have certain "universal" buttons as part of the core > theme in toolkit? This isn't super-critical because there's no overlap > because we have multiple themes in /toolkit that aren't currently shared, but > if we have a Winstripe for Tbird replacing Qute at some point, it makes sense > to have the icons in a single place. > > Or, alternatively, move the Help css into the app directory so each app skins > stuff their own way. If I haven't said this elsewhere, this would be ideal. This actually happens for some things (throbber on Mac only, for example -- there's a bug for it somewhere that I can't remember), but it doesn't for others. By the way, my computer is now back in fully, *fully* working condition. A BIOS update and some compressed air to clear out the vents did the trick, and I'm now going through the backlog of about 970-odd bugmails since mid-November, when things started failing. As I have January all but completely free to work on Mozilla-related things, this is the ideal time to get these and other such issues with Help fixed -- I just need a little help here and there with things I can't do (like reviews). I'll be working over the next couple days on a list of these things so I can make my requests for possibly expedited attention soon...
Comment 7•19 years ago
|
||
(In reply to comment #5) > > Or, alternatively, move the Help css into the app directory so each app skins > stuff their own way. sounds good to me. Proposal attached. Not too sure what to do with help.xul. I put a #ifdef in there for now but it's a bit rough. Got any idea for another solution?
Updated•19 years ago
|
Assignee: jwalden+fxhelp → rj.keller
Status: NEW → ASSIGNED
Comment 8•19 years ago
|
||
Here's the CSS file, since my CVS SSH key is long gone and can't diff new files without it.
Assignee | ||
Comment 9•19 years ago
|
||
(In reply to comment #5) > Maybe we need/want to have certain "universal" buttons as part of the core > theme in toolkit? This isn't super-critical because there's no overlap > because we have multiple themes in /toolkit that aren't currently shared, but > if we have a Winstripe for Tbird replacing Qute at some point, it makes sense > to have the icons in a single place. I think this is the only correct way to do it. App-specific Help viewer skinning is essentially forking the Help viewer CSS for every app that uses it, and as Help Viewer's part of toolkit it should be fully self-contained. (In reply to comment #7) > sounds good to me. Proposal attached. Not too sure what to do with help.xul. I > put a #ifdef in there for now but it's a bit rough. Given that creating a stash of generic buttons is the correct way to do this, and also given that this patch would introduce an app-specific #ifdef that doesn't do much to add to the utility of the Help viewer, I think this bug will have to wait for a different patch. This seems like it shouldn't be overly difficult -- maybe I'll take a look at it in a couple weeks after we fix bug 279506, which is currently a far more pressing concern.
Updated•19 years ago
|
Product: Firefox → Toolkit
Version: 1.0 Branch → unspecified
Assignee | ||
Comment 10•19 years ago
|
||
If we can get bug 295817 fixed, finishing this up should be pretty easy. The goal's to get that bug done by 1.1, and given how long the development cycle for the next release is likely to be (and also given that we're seeing more Help users), I *really* want to see a fully app-independent Help by 1.1 - one that's usable by Sunbird, xulrunner apps, and Thunderbird once it gets its act together. Barring any unexpected developments, expect to see this bug fixed in the 1.8 timeframe.
Assignee: rj.keller → jwalden+fxhelp
Status: ASSIGNED → NEW
OS: Windows 2000 → All
Hardware: PC → All
Target Milestone: --- → mozilla1.8beta3
Assignee | ||
Comment 11•19 years ago
|
||
Patch and attachment with new images coming soon...
Status: NEW → ASSIGNED
Assignee | ||
Comment 12•19 years ago
|
||
The format of a new toolbar image is as follows: +--------------+-----------------+--------------+---------------+ | Back/normal | Forward/normal | Home/normal | Print/normal | +--------------+-----------------+--------------+---------------+ | Back/hover | Forward/hover | Home/hover | Print/hover | +--------------+-----------------+--------------+---------------+ | Back/disable | Forward/disable | Home/disable | Print/disable | +--------------+-----------------+--------------+---------------+ Home/disable and Print/disable are currently unused, but I think avoiding any possible future hassle is worth the extra cost of leaving them in place instead of just marking their areas transparent. The size of each tile is dependent upon the theme itself. The burden from such an image is between 10-20K depending on the size of the buttons, which I think is acceptable.
Assignee | ||
Comment 13•19 years ago
|
||
This patch has bits of bug 296012 mixed in it, and it'll conflict with the patch in bug 299710. I'll wait until those are wrapped up before requesting review (on a new version of the patch, probably). The patch requires the images in attachment 188718 [details] to be in the tree for it to work properly. I haven't been able to get a build environment working in Windows, so it's untested both there and on Macs. I suspect it'll work fine there, too, because the fiddling I had to do to get Linux to work correctly also applied pretty easily to Windows. This patch patches qute in addition to the two main toolkit themes, pinstripe and winstripe. No testing whatsoever was done to qute, and the changes are only to get rid of a few ifdefs and put it in a possibly-working state whenever someone decides to use qute with Help. I don't think it's worth really doing a thorough review of the qute changes -- just skim to see whether the general idea's correct and leave it at that, if it's even worth reviewing them.
Attachment #176405 -
Attachment is obsolete: true
Attachment #176406 -
Attachment is obsolete: true
Assignee | ||
Comment 14•19 years ago
|
||
This patch should remove the last vestiges of Firefox-specificness from the help viewer, making it a proper part of toolkit. It's tested on Linux but is untested on Mac because I don't own one and Windows because I don't have a working build environment. The patch requires some mucking around with image files to work properly. To test, open attachment 188718 [details]. Inside it are three directories -- one for each theme in toolkit. Inside each directory you'll find a Toolbar.png specific to that theme containing the image the help viewer uses. Copy the Toolbar.png over the existing copy in mozilla/toolkit/themes/<theme>/help/ (where <theme> corresponds to the directory within attachment 188718 [details]). Apply the patch and rebuild to get a fully app-independent help viewer.
Attachment #188724 -
Attachment is obsolete: true
Attachment #189221 -
Flags: first-review?(mconnor)
Assignee | ||
Updated•19 years ago
|
Attachment #189221 -
Flags: first-review?(mconnor) → first-review?(kevin)
Updated•19 years ago
|
Attachment #189221 -
Flags: first-review?(kevin) → first-review+
Assignee | ||
Comment 15•19 years ago
|
||
Comment on attachment 189221 [details] [diff] [review] Final patch This should make the toolkit help viewer usable by non-Firefox applications such as Sunbird.
Attachment #189221 -
Flags: approval1.8b4?
Updated•19 years ago
|
Attachment #189221 -
Flags: approval1.8b4? → approval1.8b4+
Assignee | ||
Comment 16•19 years ago
|
||
The patch and toolbar palette image updates have been checked in, so I'm marking this FIXED.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Target Milestone: mozilla1.8beta3 → mozilla1.8beta4
Updated•8 years ago
|
Product: Toolkit → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•