689140 - Icons in task view and mail icon in "Write" button are size scaled with aero theme.

Status: RESOLVED FIXED

(Calendar :: Calendar Frontend, defect)

Description

[Decathlon]

There are a few trivial issues about icons in the winstripe aero theme.

- the monochrome icons in the toolbar buttons (task view) are scaled to 16X16 instead of the natural size 18x18;
- the same happens for the icon related to the mail item in the write button menu for the Thunderbird's main toolbar (maybe the same happens for the Mac, but I can't verify that);
- the day number text in the todaypane button needs to be moved to the left
- in task view, the position of the toolbar buttons need a little adjustment to make the same as the message-pane in mail mode (not aero theme specific).

Comment 1

[Decathlon]

Attachment: patch - v1

Icons for aero theme are 18x18, instead the others are 16x16, so I've used "width: auto" and "height: auto" directly in the file calendar-task-view.css that should cover both cases. 

For what I've understood, the aero theme is applied on Windows Vista/Seven even if the selected theme is different, e.g. the Windows Classic theme (and here Richard, I ask you a confirm because you know better than me). 
In this case the mail icon in the "Write" button (main toolbar) needs the mail-toolbar-small.png image (16x16) for Win XP (file lightning.css), instead the aero theme needs the 18x18 monochrome mail icon. Again "width: auto" and "height: auto" in lightning.css should cover both cases.

I also delete the rule |#newMsgButton-mail-menuitem .toolbarbutton-icon| because it seems useless, please Richard, correct me if I'm wrong.

For the todaypane button, I've deleted the code that I asked you to add in bug 668958. Don't know why, but without that, on Miramar the text was displaced, instead, on TB 7/8 the day number is correctly positioned only if that code is deleted. Could you confirm?

Comment 2

[Richard Marti (:Paenglab)]

Comment on attachment 562411 [details] [diff] [review]
patch - v1

All in all this patch looks good.
But some comments:

+#task-actions-toolbox .toolbarbutton-icon {
+    margin: 0 !important;
+    width: auto !important;
+    height: auto !important;

The three !important aren't needed. It's the last rule applied and no !important in previous rules needs to be overridden.

Instead of adding

+#newMsgButton-mail-menuitem  .menu-iconic-icon {
+  margin: 0 !important;
+  width: auto !important;
+  height: auto !important;
+}

in lightning.css, I would propose to change in lightning-aero.css this:

#newMsgButton-mail-menuitem {
  list-style-image: url(chrome://messenger/skin/icons/mail-toolbar.png);
  -moz-image-region: rect(0px 36px 18px 18px) !important;

to:

#newMsgButton-mail-menuitem {
  list-style-image: url(chrome://messenger/skin/icons/mail-toolbar.png);
  -moz-image-region: rect(1px 35px 17px 19px) !important;
}

(and still remove #newMsgButton-mail-menuitem .toolbarbutton-icon { rule as in your patch). This because you also corrected in this patch the icon for XP to 16px and with my proposed change the icon is also 16px for Vista/Win7. So the margin: 0 and width/height: auto aren't needed any more. If you still want to leave it, again the !important aren't needed and between #newMsgButton-mail-menuitem and .menu-iconic-icon is a space to much.

> I also delete the rule |#newMsgButton-mail-menuitem .toolbarbutton-icon|
> because it seems useless, please Richard, correct me if I'm wrong.

That's correct, in my patch for Bug 684518 I have it also removed.

> For the todaypane button, I've deleted the code that I asked you to add in
> bug 668958. Don't know why, but without that, on Miramar the text was
> displaced, instead, on TB 7/8 the day number is correctly positioned only if
> that code is deleted. Could you confirm?

I can also confirm this.

r+ when you check my comments.

Comment 3

[Decathlon]

Attachment: patch - v2

All notes in previous comment addressed.
I had forgotten the pinstripe and gnomestripe themes for the file calendar-task-view.css. I merely copied and pasted the code from winstripe because I can't test on Linux and Mac. Could you please verify that the buttons in the task toolbar get the correct position on those platforms? The review it's only for this issue.

Comment 4

[Richard Marti (:Paenglab)]

Comment on attachment 564330 [details] [diff] [review]
patch - v2

The Windows part looks good.

Can you also add on Linux in lightning.css this part

+  list-style-image: url(chrome://messenger/skin/icons/mail-toolbar-small.png);
+  -moz-image-region: rect(0px 32px 16px 16px);

On Mac this isn't possible because the small toolbarbuttons have 24px icons.

For Mac and Linux the #other-actions-box needs a -moz-margin-end: 3px; instead of a -moz-margin-end: -2px; (see image I'll attach).

Also for both. This isn't needed but doesn't harm if you leave it

+#task-actions-toolbox .toolbarbutton-icon {
+    margin: 0;
+    width: auto;
+    height: auto;

because where are only 16px icons used and no mix with 18px icons.

Comment 5

[Richard Marti (:Paenglab)]

Attachment: Comparison between mail header buttons and Task buttons with patch applied under Linux

The gap between Task buttons and border is closer than in mail part.

I see also a missing right border and wrong button text colors. But this isn't part of this bug. I'll file a new for this issues.

Comment 6

[Richard Marti (:Paenglab)]

Update for Linux:

Bug 673826 changed with the today nightly the padding-end of the toolbox from 6px to 2px. This means for Linux you should now use for the #other-actions-box a -moz-margin-end: -1px;

Mac wasn't changed. So the -moz-margin-end: 3px; is still valid.

Comment 7

[Decathlon]

Attachment: patch - v3

(In reply to Richard Marti [:paenglab] from comment #4)
> 
> Can you also add on Linux in lightning.css this part
> 
> +  list-style-image:
> url(chrome://messenger/skin/icons/mail-toolbar-small.png);
> +  -moz-image-region: rect(0px 32px 16px 16px);
> 

Done.

> 
> Also for both. This isn't needed but doesn't harm if you leave it
> 
> +#task-actions-toolbox .toolbarbutton-icon {
> +    margin: 0;
> +    width: auto;
> +    height: auto;
> 
> because where are only 16px icons used and no mix with 18px icons.

I deleted them both on pinstripe and gnomestripe.

(In reply to Richard Marti [:paenglab] from comment #6)
> ... This means for Linux you should now use for the
> #other-actions-box a -moz-margin-end: -1px;
> Mac wasn't changed. So the -moz-margin-end: 3px; is still valid.

Done.

If pinstripe or gnomestripe need some adjustment, please feel free to make them since I can't test.

Comment 8

[Richard Marti (:Paenglab)]

Comment on attachment 568638 [details] [diff] [review]
patch - v3

(In reply to Decathlon from comment #7)
> If pinstripe or gnomestripe need some adjustment, please feel free to make
> them since I can't test.

Not needed. Looks good.

r+

Comment 9

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

comm-central changeset 2b890c7316e8

Comment 10

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Backported to releases/comm-aurora changeset 672ee035ff4a

Comment 11

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Backported to releases/comm-beta changeset a5f8a5093dae