722758 - Print button is misaligned in the Mail & News toolbar with the Modern theme

Status: VERIFIED FIXED

(SeaMonkey :: Themes, defect)

Description

[rsx11m]

Steps to reproduce:

 - open the Mail/News window (new or existing profile)
 - customize the toolbar to add the "Print" button anywhere
 - this looks fine while using the Default theme
 - switch to Modern theme, now it is too low and misaligned

> Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0.1) Gecko/20111221 SeaMonkey/2.6.1
> Mozilla/5.0 (X11; Linux x86_64; rv:12.0a1) Gecko/20120131 SeaMonkey/2.9a1

Comment 1

[rsx11m]

Attachment: Appearance on Windows 7

with Windows Classic desktop theme.

Comment 2

[rsx11m]

That's actually a quite recent regression. I've created a new profile with 2.1 and added the Print button to the Mail/News toolbar - looks fine even with the Modern theme. From there I went to 2.5b4 and the Print button was still there and well aligned.

Now, when going from 2.5b4 to 2.6.1, the Print button disappeared from the previously customized toolbar, and when adding it back from the customization palette, it shows the misalignment as described. Thus, the regression happened somewhere between 2.5 and 2.6.

Comment 3

[rsx11m]

Ian, possibly related to bug 676991 "Move toolbar print button to utilityOverlay"? The patch in attachment 553967 [details] [diff] [review] made quite a few changes to the style files and would fit the time frame.

Comment 4

[Philip Chee]

We switched from using:
#button-print {
  list-style-image: url("chrome://communicator/skin/icons/btn1.gif");
  -moz-image-region: rect(34px 49px 67px 0);
}

To
#print-button {
  list-style-image: url("chrome://communicator/skin/icons/common.png");
  -moz-image-region: rect(0 42px 39px 0);
}

The height changed from 33px to 39px.

Hg Blame says:
http://hg.mozilla.org/comm-central/rev/a508ab7862d2
author	Ian Neal <[e-mail]>
Bug 676991 - Move toolbar print button to utilityOverlay r=neil/standard8

Comment 5

[rsx11m]

I'm measuring six pixels off in the screen shot, so this would match that difference. Bottom line, the image is too big now, or could it just be cropped?

On a side note, it looks (at least visually) correct in the main toolbar of the navigator window.

Comment 6

[rsx11m]

In a naive approach based on Phil's comment #4, I've changed all -moz-image-region: rect(0 * 39px *); to rect(3px * 36px *); in communicator.css, which indeed resolves the problem for the mail/news toolbar. However, given that print-button is shared among windows now (the aim of bug 676991 to start with) it looks horribly misaligned in the main-window toolbar with that change. Thus, adjusting this globally doesn't seem to be the right way to go... :-\

Comment 7

[Ian Neal]

Attachment: Temporary workaround until new icons arrive [Checked in:comm-central Comment 16 comm-aurora/beta Comment 17]

This patch:
* overrides the print icon for mailnews windows

Hopefully when there is a new modern theme the icons in all components will be the same size as they are for classic plus there will be small versions for mailnews.

Comment 8

[Philip Chee]

Comment on attachment 593257 [details] [diff] [review]
Temporary workaround until new icons arrive [Checked in:comm-central Comment 16 comm-aurora/beta Comment 17]

Review of attachment 593257 [details] [diff] [review]:
-----------------------------------------------------------------

Cancelling review request for the time being.

::: suite/themes/modern/messenger/primaryToolbar.css
@@ +264,5 @@
>  #button-junk[disabled="true"] {
>    -moz-image-region: rect(544px 199px 577px 150px) !important;
>  }
>  
> +/* To workaround the mailnews icons are 33px tall and have no small versions */

We *were* using the small print buttons from chrome://communicator/skin/icons/common-small.png !
http://hg.mozilla.org/comm-central/rev/a508ab7862d2#l20.46

Could you put these back? Thanks.

Comment 9

[Philip Chee]

> +toolbar[iconsize="small"] > #print-button[disabled="true"],
> +#print-button[disabled="true"] {
> +  -moz-image-region: rect(3px 168px 36px 126px) !important;
What style are your overriding with !important here?

Comment 10

[Stefan [:stefanh]]

(In reply to Philip Chee from comment #9)
> > +toolbar[iconsize="small"] > #print-button[disabled="true"],
> > +#print-button[disabled="true"] {
> > +  -moz-image-region: rect(3px 168px 36px 126px) !important;
> What style are your overriding with !important here?

iirc it's the hover:active rules (it's like this in all files). We don't use not:([disabled="true"]) etc

Comment 11

[rsx11m]

(In reply to Philip Chee from comment #8)
> We *were* using the small print buttons

Other than the "Stop" button (apparently due to the same global definition as the "Print" button now) and changes in the spacing, the icons stay the same in the main Mail & News as well as the composition window when switching to small icons, so it seems to be more consistent to not use small icons for print-button either (and likely so for the stop-button, but that may be a separate bug).

Comment 12

[Philip Chee]

Comment on attachment 593257 [details] [diff] [review]
Temporary workaround until new icons arrive [Checked in:comm-central Comment 16 comm-aurora/beta Comment 17]

OK. I'll give the patch a spin.

Comment 13

[Philip Chee]

Comment on attachment 593257 [details] [diff] [review]
Temporary workaround until new icons arrive [Checked in:comm-central Comment 16 comm-aurora/beta Comment 17]

The print button is much better aligned. Compared to 2.0.14 it's about a couple of pixels lower. Moving the -moz-image-region 2 px down seems to work better. e.g.

-  -moz-image-region: rect(3px 42px 36px 0); 
+  -moz-image-region: rect(5px 42px 38px 0);

r=me with the 2px shift at your discretion.

Comment 14

[rsx11m]

Comment on attachment 593257 [details] [diff] [review]
Temporary workaround until new icons arrive [Checked in:comm-central Comment 16 comm-aurora/beta Comment 17]

(In reply to Philip Chee from comment #13)
> +  -moz-image-region: rect(5px 42px 38px 0);

I've patched 2.7 with this modification and it looks good to me as well.

Comment 15

[Ian Neal]

Comment on attachment 593257 [details] [diff] [review]
Temporary workaround until new icons arrive [Checked in:comm-central Comment 16 comm-aurora/beta Comment 17]

It would be good to get this into 2.8 and 2.9 too

Comment 16

[Ian Neal]

Comment on attachment 593257 [details] [diff] [review]
Temporary workaround until new icons arrive [Checked in:comm-central Comment 16 comm-aurora/beta Comment 17]

http://hg.mozilla.org/comm-central/rev/2896ec0b8c06

Comment 17

[Ian Neal]

Comment on attachment 593257 [details] [diff] [review]
Temporary workaround until new icons arrive [Checked in:comm-central Comment 16 comm-aurora/beta Comment 17]

http://hg.mozilla.org/releases/comm-aurora/rev/a4536456e92a
http://hg.mozilla.org/releases/comm-beta/rev/458dad0ffd8f

Comment 18

[rsx11m]

Verified fixed on Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0a2) Gecko/20120208 SeaMonkey/2.9a2, thanks!