Closed Bug 426744 Opened 14 years ago Closed 14 years ago

Style print preview window toolbar on Windows

Categories

(Firefox :: Theme, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 3

People

(Reporter: faaborg, Unassigned)

References

Details

Attachments

(3 files, 2 obsolete files)

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

I don't know, sorry. I only found out recently...
If you happen to find where that color gets set, can you change it to the system dialog box color for both cases?
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 ;)
(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.
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.
Attached image -moz-appearance: browsertabbar-toolbox (obsolete) —
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...
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).
-moz-appearance: toolbox seems right for this.
Attachment #318145 - Flags: review?(gavin.sharp)
Attachment #318145 - Flags: review?(gavin.sharp) → review+
Attachment #318145 - Flags: approval1.9?
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 on attachment 318145 [details] [diff] [review]
style the toolbar with the toolbox appearance

a1.9+=damons
Attachment #318145 - Flags: approval1.9? → approval1.9+
... 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.
Attachment #318138 - Attachment is obsolete: true
Attachment #318145 - Attachment is obsolete: true
Keywords: checkin-needed
Assignee: nobody → dao
Attachment #318214 - Flags: review?(gavin.sharp)
The second half of the patch needs review.
Keywords: checkin-needed
Comment on attachment 318214 [details] [diff] [review]
style the toolbar with the toolbox appearance (checked in)

eh, no it doesn't.
Attachment #318214 - Flags: review?(gavin.sharp) → review+
Depends on: 431309
This bug needs to stay open after that patch has landed.
Keywords: checkin-needed
mozilla/browser/themes/winstripe/browser/browser-aero.css 	1.14
mozilla/browser/themes/winstripe/browser/browser.css 	1.217 
Status: NEW → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3
Attachment #318214 - Attachment description: style the toolbar with the toolbox appearance → style the toolbar with the toolbox appearance (checked in)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee: dao → nobody
Status: REOPENED → NEW
Target Milestone: Firefox 3 → ---
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.
A followup bug would look just like this one since nothing of what comment 0 proposed is done.
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.
Status: NEW → RESOLVED
Closed: 14 years ago14 years ago
No longer depends on: 431309
Resolution: --- → FIXED
Summary: Style print preview window on Windows → Style print preview window toolbar on Windows
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?
Target Milestone: --- → Firefox 3
(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
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.
(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).
(In reply to comment #26)
> being pushed off to a future release (see comment 23).
> 

s/23/22/  Sorry for the spam.
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.