345609 - [FIX]Themes can no longer style print preview scrollbars anything but "native"

Status: RESOLVED FIXED

(Core :: Print Preview, defect, P2)

Description

[Peter Weilbacher]

What I already noted before on Firefox is also true for SeaMonkey on trunk: the scrollbars, scrollbarbuttons, and the scrollbar corner are not themed any more. That means they appear white.

As far as I know this is OS/2 only, because nobody replied to my multiple posts in the newsgroup about this.

I now discovered some old nightlies and determined that the regression occurred between 2006012505 and 2006012605, so I searched on bonsai like this (giving it a few more hours as timezone buffer):
http://bonsai.mozilla.org/cvsquery.cgi?module=SeaMonkeyBrowser&date=explicit&mindate=2006-01-25+01&maxdate=2006-01-26+08

Unfortunately, that didn't turn up something that to me immediately looked like a possible cause and still leaves quite a few big checkins in the possible range.

Comment 1

[Peter Weilbacher]

roc, your large checkin for Bug 317375 falls into that timeframe. Do you think that one of the changes could have caused this problem back then? I see one checkin to nsGfxScrollFrame.cpp that even has some comments about painting scrollbars and scrollcorner... If you have any hints where I could put some debug output, please let me know.

Comment 2

[Stephen Donner [:stephend] Not actively reading bugmail]

Happens on WinXP, too.  I thought this was known (and filed), and just ignored because it was SeaMonkey, but here's hope!

Comment 3

[Peter Weilbacher]

Huh, there is bug 344039 (which I obviously was too tired to find last night) but that explicitely says that it only occurs in Thunderbird.

Comment 4

[Jason Bassford]

I don't see this.  I'm using a SeaMonkey 7/22 build and XP.  Under what SeaMonkey theme is this happening?  

(I filed bug 187916 three and a half years ago because Modern doesn't theme scrollbars at all - which even though it themes all other UI elements - but I don't think this can possibly be what you're referring to because there's no regression involved in that.)

Comment 5

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Modern

Comment 6

[Jason Bassford]

Ah!  The screenshot clears everything up.  It's not that it's no longer themed per the OS theme, it's that it's no longer themed per the Modern SeaMonkey theme.

Comment 7

[Peter Weilbacher]

*** Bug 344039 has been marked as a duplicate of this bug. ***

Comment 8

[[e-mail]]

It looks like this issue is caused by the "Print background" option - if you turn that on then scrollbars display properly. Is there any way for chrome CSS to override that option or will this need to be fixed in core code?

Note that the Classic and *Stripe themes use nsITheme to draw their backgrounds and therefore do not appear to be affected, except for the scroll corner.

Comment 9

[[e-mail]]

(In reply to comment #8)
>Note that the Classic and *Stripe themes use nsITheme to draw their backgrounds
Well, not on some platforms, e.g. OS/2

Comment 10

[Boris Zbarsky [:bzbarsky]]

I bet the issue is that nsBoxFrame::Paint used to not follow the "paint background" setting, but now it just uses nsDisplayBackground to paint.  This does call HonorPrintBackgroundSettings() on the frame, but all frames return PR_TRUE.  Should nsBoxFrame be returning false?

Comment 11

[Boris Zbarsky [:bzbarsky]]

Oh, and I see no way for chrome to affect this behavior.

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Or maybe just nsScrollbarFrame and nsSliderFrame, although there's still the issue of the buttons.

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Perhaps nsBoxFrame::HonorPrintBackgroundSettings() should return false for frame descendants of the print preview viewport scrollbars.

Comment 15

[Alfred Kayser]

This is not just about 'no background', but all themes not using the -moz-appearance have the problem of wrongly sized scrollbars...

Comment 16

[Alfred Kayser]

See also:
https://bugzilla.mozilla.org/attachment.cgi?id=262344

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Works for me on Linux, 2007-10-06-04-trunk nightly and pretty current debug build.

Comment 18

[Boris Zbarsky [:bzbarsky]]

David, are you using a non-default theme?  And do you have background printing disabled?  This problem only affects themes that don't use native-themed widgets.

Comment 19

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Er, sorry, never mind.  Somebody who understands what the bug is should update the summary, though, since it's currently wrong.

Comment 20

[Damon Sicore (:damons)]

Given the settings mentioned in Comment #18, should this really block beta?  

Comment 21

[Boris Zbarsky [:bzbarsky]]

Damon, the "background printing disabled" setting is the default.

For the other... this will break with any theme not using OS-appearance scrollbar styling.  On operating systems where we have no OS-appearance styling (e.g. OS/2, where the bug was first encountered) it breaks with all themes.  Whether you're willing to block beta for it is a good question, but the fix will have to come in core code, not on the theme side.

Frankly, I think we should just return false for any frame whose content is in a native anonymous subtree.  That might be an easier test than "descendants of the print preview viewport scrollbars", but will also keep the styling on scrollbars in the document itself.  Not sure whether that's desirable.

Comment 22

[Robert O'Callahan (:roc) (email my personal email if necessary)]

That probably is desirable, so I agree that's the right approach...

Comment 23

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This should definitely not block beta! I don't know how that happened.

Comment 24

[Damon Sicore (:damons)]

OK.  Thanks roc.  Targeting for the next release.

Comment 25

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I'm not even sure it should block 1.9, to be honest. But it's relatively easy to fix so we should probably still fix it.

Comment 26

[Damon Sicore (:damons)]

Minusing.

Comment 27

[Boris Zbarsky [:bzbarsky]]

We need a separate bug on comment 15, since it doesn't have anything to do with the problem this bug is about.

Comment 28

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch per comment 21

Comment 29

[Alfred Kayser]

Regarding comment 15 and comment 27: See bug 378272.
The issue may be very much in the same direction. 
That in the print-preview display, the non-native scrollbars get some scaling applied that just like the suppression of background-color should not happen.

Comment 30

[Boris Zbarsky [:bzbarsky]]

Checked in.