Closed Bug 316851 Opened 15 years ago Closed 15 years ago

Need workaround for bug 56488 in Help

Categories

(SeaMonkey :: Help Documentation, defect)

PowerPC
macOS
defect
Not set
normal

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+
iann_bugzilla
: 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.
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?
Here's a screenshot of how it would look on Mac (both themes).
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).
I suppose I better take this since Neil doesn't have a mac ;)
Status: NEW → ASSIGNED
Another approach that won't affect other systems.
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.
Here's screenshots of how it currently looks on mac. Note that the down-scroll arrow is covered by the resizer.
Assignee: neil.parkwaycc.co.uk → stefan_h
Status: ASSIGNED → NEW
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?
(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)
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...)
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.
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.
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.
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.
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)
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]
Attachment #204573 - Attachment is obsolete: false
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.
Re your border issue, would that be solved if you used a real <statusbar/>?
(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 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+
(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?
(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?
> 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.
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.
This version sets the height of the hbox with css. That will also make the hbox themable.
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)
Attachment #204637 - Flags: review?(jag)
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!
 > 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.

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)
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).
Attachment #204899 - Attachment mime type: text/plain → image/gif
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 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+
Attachment #205079 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
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 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 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+
Checked in on trunk/branch by Standard8.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
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 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.