426744 - Style print preview window toolbar on Windows

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Alex Faaborg [:faaborg] (Firefox UX)]

Attachment: Screen shot of the current print preview window

The print preview window could use some polish:

-Use a dialog box color for the background instead of window to create better contrast between the background and the page

-Update the shadows to use an alpha channel and to be a little larger

-Make the edges of the page crisper

Comment 1

[Lim Chee Aun]

The only way I could polish it is this:

#print-preview-toolbar{
-moz-appearance: browsertabbar-toolbox !important;
}

I couldn't find where to style the 'page'.

Comment 2

[Daniel.S]

The non-page-background becomes dark grey when you choose "Page Setup..." and tick the option to print background images and colors.

Comment 3

[Alex Faaborg [:faaborg] (Firefox UX)]

>The non-page-background becomes dark grey when you choose "Page Setup..." and
>tick the option to print background images and colors.

Yeah, you're right, why?

Comment 4

[Daniel.S]

(In reply to comment #3)
> Yeah, you're right, why?

I don't know, sorry. I only found out recently...

Comment 5

[Alex Faaborg [:faaborg] (Firefox UX)]

If you happen to find where that color gets set, can you change it to the system dialog box color for both cases?

Comment 6

[Daniel.S]

The dark background color is #737373 which only appears in http://mxr.mozilla.org/mozilla/source/layout/style/ua.css#212

The bright color isn't mentioned here, and since it's #fff it's nothing you could find easily in MXR.

Oh, I'm not able to write patches. I'm not that deep into Mozilla, yet ;)

Comment 7

[Dão Gottwald [:dao]]

(In reply to comment #3)
> >The non-page-background becomes dark grey when you choose "Page Setup..." and
> >tick the option to print background images and colors.
> 
> Yeah, you're right, why?

Guess: Because the implementation disables all background colors for the given document which seems to include the pages container, or something like that. I haven't actually looked at the code.

Comment 8

[Wesley Johnston (:wesj)]

The white background, page border, and the shadow color are hard coded in:

http://mxr.mozilla.org/mozilla/source/layout/generic/nsPageFrame.cpp#451

Maybe those should be added to the list of hard coded colors? Changing them is (relatively) easy. Converting to CSS is probably hard.

Comment 9

[Kai Liu]

Attachment: -moz-appearance: browsertabbar-toolbox

It might be a good idea to just take the suggestion made in comment 1, which would restore the missing bottom border to the toolbar in themed mode (see attached before-and-after screenshot), and then save the rest for a future release...

Comment 10

[Dão Gottwald [:dao]]

There's no -moz-appearance: browsertabbar-toolbox on Windows XP. What you see in attachment 318138 [details] is the classic fallback styling (-moz-appearance: none).

Comment 11

[Dão Gottwald [:dao]]

-moz-appearance: toolbox seems right for this.

Comment 12

[Dão Gottwald [:dao]]

Attachment: style the toolbar with the toolbox appearance

Comment 13

[Alex Faaborg [:faaborg] (Firefox UX)]

Attachment: Print preview toolbar on vista

I recommend that we go with -moz-appearance: browsertabbar-toolbox since the vertically centered reflection line looks better with Aero (lines up with all of the buttons and the drop down).  If we don't trust have windows fall back for classic I guess we can have different styles based on default and non-default os themes.

Comment 14

[Damon Sicore (:damons)]

Comment on attachment 318145 [details] [diff] [review]
style the toolbar with the toolbox appearance

a1.9+=damons

Comment 15

[Dão Gottwald [:dao]]

Attachment: style the toolbar with the toolbox appearance (checked in)

... and browsertabbar-toolbox for Vista.

FWIW, the higher reflection is by design, as it doesn't distort the text. We accepted that distortion on the tabs, though, and I agree that it looks better with the buttons.

Comment 16

[Reed Loden [:reed]]

The second half of the patch needs review.

Comment 17

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 318214 [details] [diff] [review]
style the toolbar with the toolbox appearance (checked in)

eh, no it doesn't.

Comment 18

[Dão Gottwald [:dao]]

This bug needs to stay open after that patch has landed.

Comment 19

[:Gavin Sharp [email: [e-mail]]]

mozilla/browser/themes/winstripe/browser/browser-aero.css 	1.14
mozilla/browser/themes/winstripe/browser/browser.css 	1.217 

Comment 20

[:Gavin Sharp [email: [e-mail]]]

Oops, didn't see your comment, Dao. What is left to be done? Would be best to file a followup bug on that, I think - landing multiple separate patches in one bug is confusing.

Comment 21

[Dão Gottwald [:dao]]

A followup bug would look just like this one since nothing of what comment 0 proposed is done.

Comment 22

[:Gavin Sharp [email: [e-mail]]]

Well, then perhaps it would have been best to file a different bug on making the change we did. Either way, someone looking at blame at some point in the future who wants to figure out why we added the -moz-appearance rule to the toolbar won't want to wade through a bunch of comments about making the changes in comment 0.

I filed bug 432479.

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+2]]

So whats happened with the dark gray background around the page? Now it is totally white and doesn't look very nice. It makes it hard to identify the real page even there is a border around. Will this be reverted?

Comment 24

[Kai Liu]

(In reply to comment #23)
> So whats happened with the dark gray background around the page? Now it is
> totally white and doesn't look very nice. It makes it hard to identify the real
> page even there is a border around. Will this be reverted?
> 

See comment #2

Comment 25

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Why does it depend on this setting? I cannot see a coherence in printing backgrounds or not. The gray area is outside of the print preview of the page.

Comment 26

[Kai Liu]

(In reply to comment #25)
> Why does it depend on this setting? I cannot see a coherence in printing
> backgrounds or not. The gray area is outside of the print preview of the page.
> 
The subsequent comments (esp. 7 & 8) explain.  All that this patch does is restyle the toolbar.  This background color thing has been around for a long time, and it's too late to make the sort of change needed to fix that, so it's being pushed off to a future release (see comment 23).

Comment 27

[Kai Liu]

(In reply to comment #26)
> being pushed off to a future release (see comment 23).
> 

s/23/22/  Sorry for the spam.

Comment 28

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thanks Kai. The comments were a bit confusing.

Verified with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008050606 Minefield/3.0pre ID:2008050606