Closed
Bug 316851
Opened 19 years ago
Closed 19 years ago
Need workaround for bug 56488 in Help
Categories
(SeaMonkey :: Help Documentation, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
seamonkey1.0beta
People
(Reporter: stefanh, Assigned: stefanh)
Details
(Keywords: fixed1.8)
Attachments
(9 files, 5 obsolete files)
118.29 KB,
image/gif
|
Details | |
46.89 KB,
image/gif
|
Details | |
114.92 KB,
image/gif
|
Details | |
119.54 KB,
image/gif
|
Details | |
33.07 KB,
image/gif
|
Details | |
25.18 KB,
image/gif
|
Details | |
116.51 KB,
image/gif
|
Details | |
37.77 KB,
image/gif
|
Details | |
3.62 KB,
patch
|
jag+mozilla
:
review+
neil
:
superreview+
iannbugzilla
:
approval-seamonkey1.0+
|
Details | Diff | Splinter Review |
Currently, because of bug 56488, mac user can't use the down scroll arrow in the Help window - it's hidden behind the resizer. Toolkit have work around this by adding margin to #appcontent in Pinstripe's help.css. We can't do this, though - doing that would affect all platforms. One approach is to use platformClasses.css to insert a fake statusbar at the bottom of the window on Mac.
Assignee | ||
Comment 1•19 years ago
|
||
This is one way of doing it. The problem is that I don't know the effects of the border-style on other platforms. Another way of doing it would be to add the <hbox/> to just the content window - then the effect would be similar to toolkit's Help window. Thoughts?
Assignee | ||
Comment 2•19 years ago
|
||
Here's a screenshot of how it would look on Mac (both themes).
Assignee | ||
Comment 3•19 years ago
|
||
Screenshot of Firefox Help window for comparison. Note that it's only the content area that has the faked statusbar - done with css (not possible in SeaMonkey).
Assignee | ||
Comment 4•19 years ago
|
||
I suppose I better take this since Neil doesn't have a mac ;)
Status: NEW → ASSIGNED
Assignee | ||
Comment 5•19 years ago
|
||
Another approach that won't affect other systems.
Assignee | ||
Comment 6•19 years ago
|
||
This is probably the best alternative. The only thing that keeps it from being perfect in Modern is the splitter's right border. But I guess it's better than not being able to scroll the Help window.
Assignee | ||
Comment 7•19 years ago
|
||
Here's screenshots of how it currently looks on mac. Note that the down-scroll arrow is covered by the resizer.
Assignee | ||
Updated•19 years ago
|
Assignee: neil.parkwaycc.co.uk → stefan_h
Status: ASSIGNED → NEW
Comment 8•19 years ago
|
||
So the Mac resizer is always native coloured, right? How does that look in the Modern theme in other windows? Would it be possible to make the hbox match?
Assignee | ||
Comment 9•19 years ago
|
||
(In reply to comment #8) > So the Mac resizer is always native coloured, right? How does that look in the > Modern theme in other windows? Would it be possible to make the hbox match? I assume you mean matching the statusbar in other windows? As you can see, apart from a missing 3d effect, the hbox matches pretty well. The MailNews statusbar lacks a top border (also no border in, for instance, bm manager and View Source). In the MailNews case I assume a border would look strange when you have focus in the content area (black border around the area, in the image left pane has focus)
Comment 10•19 years ago
|
||
Ok, so maybe this is silly, but what about adding this style to the XP_MACOSX section of platformClasses.css: :root { overflow: scroll; } (I think :root is correct but bz wasn't around to check...)
Assignee | ||
Comment 11•19 years ago
|
||
Code-wise, it's a clean solution. But if you judge it by the way it looks, then I think my solution is better. "Using overflow: scroll;" looks acceptable in Classic. Modern looks worse with it's "empty" scrollbar with arrows. Note also that there will be an empty vertical scrollbar in some windows.
Comment 12•19 years ago
|
||
What you really want is just a little block below the down scroll arrow that pushes the scrollbar up (or rather, compresses it) but doesn't affect the content area. Naturally we don't want to apply this to all vertical scrollbars on Mac, just this specific instance, but I'm not quite sure how we get our special version of vertical scrollbar applied.
Comment 13•19 years ago
|
||
Of course, providing a true fix kinda defeats the purpose of this work-around bug, and I'm guessing the real fix is hard. As a work-around I don't really care whether it's a fake status bar or a disabled horizontal scrollbar, though I'd like to see the fake status bar run below everything, not just the content area.
Comment 14•19 years ago
|
||
Which would be the first patch, more of less. I think I can live without the border between the status bar and the content pane.
Assignee | ||
Comment 15•19 years ago
|
||
OK, hope you're OK with this Neil. I just added height="15" to the <hbox/> in help.xul. No need to use the appearance stuff in classic either. So, this patch only adds a 3 lines of code to help.xul. I couldn't help removing some empty lines at the end of the file, so we actually end up with -2 lines...
Attachment #203400 -
Attachment is obsolete: true
Attachment #204308 -
Attachment is obsolete: true
Attachment #204573 -
Attachment is obsolete: true
Attachment #204637 -
Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #204637 -
Flags: review?(jag)
Assignee | ||
Comment 16•19 years ago
|
||
Here's how it looks. Interestingly, in classic, there's a gap between the <hbox/> and the down-scroll arrow that looks 0.5 px wider than normal (if I look at other windows). But it is probably correct, looks the same as the one in attachment #204573 [details]
Assignee | ||
Updated•19 years ago
|
Attachment #204573 -
Attachment is obsolete: false
Comment 17•19 years ago
|
||
I'm not feeling too happy with hardcoding that height in the XUL, though I guess it's directly tied to the height of the resizer widget.
Comment 18•19 years ago
|
||
Re your border issue, would that be solved if you used a real <statusbar/>?
Assignee | ||
Comment 19•19 years ago
|
||
(In reply to comment #17) > I'm not feeling too happy with hardcoding that height in the XUL, though I > guess it's directly tied to the height of the resizer widget. Ah, ok. I'll do it with css if you prefer - I just thought it wasn't necessary to touch two more files. (In reply to comment #18) > Re your border issue, would that be solved if you used a real <statusbar/>? You mean the fact that the latest approach means that there's no border anymore? No it won't - since I believe we only want a border at the bottom of the content area, like in attachment #203401 [details]
Status: NEW → ASSIGNED
Target Milestone: --- → seamonkey1.0beta
Comment 20•19 years ago
|
||
Comment on attachment 204637 [details] [diff] [review] New version, following jag's suggestions OK, so this full-width "statusbar" resembles the effect you get with the mail window with the missing border anyway.
Attachment #204637 -
Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
Comment 21•19 years ago
|
||
(In reply to comment #17) >I'm not feeling too happy with hardcoding that height in the XUL, though I >guess it's directly tied to the height of the resizer widget. Would a real <statusbar> already have the correct height?
Comment 22•19 years ago
|
||
(In reply to comment #20) > (From update of attachment 204637 [details] [diff] [review] [edit]) > OK, so this full-width "statusbar" resembles the effect you get with the mail > window with the missing border anyway. That's what I was going for, consistency in look with other windows (e.g. Mail/News, Browser with sidebar). Stefan, wanna try a true <statusbar> all the way across and see how it looks?
Assignee | ||
Comment 23•19 years ago
|
||
> Stefan, wanna try a true <statusbar> all the way across and see how it looks?
I think I already tried that when I started to look at the problem, but I can try again when I get back home (no equipment and time here) - I don't remember exactly how it looked. But I think the height of the statusbar would be the height set in global/mac/global.css. For classic it's 1.9em and for Modern no height is set iirc (that would explain why the statusbar's height in Modern isn't enough to reach the top of the resizer in some windows - like the bm window - no bugs filed yet, but if I'm right we should probably set a min-height to 15px. I actually get the feeling that the mac resizer just floats above everything.
Assignee | ||
Comment 24•19 years ago
|
||
Yeah, it works like I said. If you use a <statusbar/> in help.xul, without specifying any height the following happens: Modern: No visible statusbar in the Help window. Classic: A statusbar with the same height as in the other windows. Now, if you think it's OK to set the min-height of the statusbar in modern/global/global.css to 15px we could go for it. That would also make the Modern statusbar look ok in, for instance, the bm window. That is, I think the Mac Classic statusbar is too high to look nice in the Help window, but on the other hand it is too high in a lot of other windows. So, the height of the mac classic statusbar should imo be changed anyway (in a separate bug I suppose). It also seems that the current em unit causes text to render badly in the statusbar on at least 10.3.9 - I always have a lot of odd text artifacts in my chatzilla statusbar. It seems that those artifacts doesn't appear if I change the height unit to pixels.
Assignee | ||
Comment 25•19 years ago
|
||
This version sets the height of the hbox with css. That will also make the hbox themable.
Assignee | ||
Comment 26•19 years ago
|
||
Comment on attachment 204834 [details] [diff] [review] New version with height set in help.css jag, do you like this one better (no hard-coded height)?
Attachment #204834 -
Flags: review?(jag)
Assignee | ||
Updated•19 years ago
|
Attachment #204637 -
Flags: review?(jag)
Comment 27•19 years ago
|
||
That's better, but I like your previous suggestion even better than that! Let's fix up the <statusbar/> CSS in classic and modern and use that. I'm ok with you doing that under this bug, but if you feel another bug needs to be filed, go ahead and make this depend on that. Btw, if we wanna keep the old <statusbar/> height in classic for whatever reason, we can always override it specifically for Help. Thank you for working on this!
Assignee | ||
Comment 28•19 years ago
|
||
> Let's fix up the <statusbar/> CSS in classic and modern and use that. I'm ok
> with you doing that under this bug, but if you feel another bug needs to be
> filed, go ahead and make this depend on that.
>
> Btw, if we wanna keep the old <statusbar/> height in classic for whatever
> reason, we can always override it specifically for Help.
OK, Modern is pretty straight-forward. 15px min-height in global.css and "border-top: none" in help.css. Classic is a bit more complicated, the statusbar has a min-height of 1.9em in mac/global.css. I could override that in help.css by setting min-height to 15px. As it is now, using min-height 15px in global.css makes us lose one pixel.
Statusbar in navigator window:
1) With min-height set to 1.9em in global.css: Computed style of height is 18.9333px and computed min-height is 20.9333px
2) With min-height set to 15px in global.css: Computed style of height is 18px and (naturally) min-height is 15px
Hmm, I do want to get rid of that em unit, since I think it's the cause of other issues... One solution is to set the height of the statusbar to 21px in global.css and to 15px in help.css. Interestingly, that would change the position of the buttons in the component bar - they will be positioned 1px closer to the top.
Assignee | ||
Comment 29•19 years ago
|
||
OK, so I think I have a solution that works well here. Statusbar in both themes global.css gets a min-height of 15px. With this approach we only need to remove the statusbar's top border in Modern's help.css. Now, in order to compensate the loss of 1px height in Classic's statusbar, I've added 1px top padding to the statusbarpanels. The statusbar's height is now 19px instead of 18.9333px. Height of statusbar and vertical position of elements looks identical with/without the patch. Note that this is only tested on mac, I don't know the effects of the min-height: 15px; in modern/global/global.css on nix/win. jag, what do you think?
Attachment #204637 -
Attachment is obsolete: true
Attachment #204834 -
Attachment is obsolete: true
Attachment #204898 -
Flags: review?(jag)
Attachment #204834 -
Flags: review?(jag)
Assignee | ||
Comment 30•19 years ago
|
||
Here's a screenshot of the navigator statusbar (non-patched vs patched build). Never mind the lack of rightmost part of the content area's bottom-border in the non-patched build - that is a different issue and you'll see it in the patched build as well (but not in this particular case).
Assignee | ||
Updated•19 years ago
|
Attachment #204899 -
Attachment mime type: text/plain → image/gif
Assignee | ||
Comment 31•19 years ago
|
||
jag thought it was best to leave the 1.9em min-height in Classic's global.css and file a separate bug on that. I'll probably do that when I've done some more investigation of the artifact issue.
Attachment #204898 -
Attachment is obsolete: true
Attachment #205079 -
Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #205079 -
Flags: review?(jag)
Attachment #204898 -
Flags: review?(jag)
Comment 32•19 years ago
|
||
Comment on attachment 205079 [details] [diff] [review] New version - don't touch classic's global.css (checked in to trunk/branch 1.8 & 1.8.0) Wow, sorry, I thought I'd already marked this r=jag
Attachment #205079 -
Flags: review?(jag) → review+
Updated•19 years ago
|
Attachment #205079 -
Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
Assignee | ||
Comment 33•19 years ago
|
||
Comment on attachment 205079 [details] [diff] [review] New version - don't touch classic's global.css (checked in to trunk/branch 1.8 & 1.8.0) Fixes a long-time standing mac issue in the Help viewer (down-scroll arrow hidden behind resizer). Low-risk, mac-only...
Attachment #205079 -
Flags: approval-seamonkey1.0?
Comment 34•19 years ago
|
||
Comment on attachment 205079 [details] [diff] [review] New version - don't touch classic's global.css (checked in to trunk/branch 1.8 & 1.8.0) a=me for branch checkin, still one needed to go
Comment 35•19 years ago
|
||
Comment on attachment 205079 [details] [diff] [review] New version - don't touch classic's global.css (checked in to trunk/branch 1.8 & 1.8.0) a=me - the second one!
Attachment #205079 -
Flags: approval-seamonkey1.0? → approval-seamonkey1.0+
Assignee | ||
Comment 36•19 years ago
|
||
Checked in on trunk/branch by Standard8.
Assignee | ||
Updated•19 years ago
|
Attachment #205079 -
Attachment description: New version - don't touch classic's global.css → New version - don't touch classic's global.css (checked in to trunk/branch)
Comment 37•19 years ago
|
||
Comment on attachment 205079 [details] [diff] [review] New version - don't touch classic's global.css (checked in to trunk/branch 1.8 & 1.8.0) Checking in (branch 1.8.0) extensions/help/resources/content/help.xul; new revision: 1.59.14.1; previous revision: 1.59 extensions/help/resources/skin/classic/help.css; new revision: 1.16.14.1; previous revision: 1.16 extensions/help/resources/skin/modern/help.css; new revision: 1.15.14.1; previous revision: 1.15 themes/modern/global/global.css; new revision: 1.51.34.1; previous revision: 1.51 done
Attachment #205079 -
Attachment description: New version - don't touch classic's global.css (checked in to trunk/branch) → New version - don't touch classic's global.css (checked in to trunk/branch 1.8 & 1.8.0)
You need to log in
before you can comment on or make changes to this bug.
Description
•