1172241 - Get rid of messageWindow-aero.css

Status: RESOLVED FIXED

(Thunderbird :: Theme, defect)

Description

[Richard Marti (:Paenglab)]

Like FX I'm working on removing the -aero files. This one is the first bug for this.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: unifyMessageWindow.patch

This patch combines both messageWindow.css files to one file and uses media queries to differentiate.

Like FX I also rename the XP image to -XP and remove the -aero from the aero image.

Comment 2

[Richard Marti (:Paenglab)]

(In reply to Richard Marti (:Paenglab) from comment #1)
> Created attachment 8616373 [details] [diff] [review]
> unifyMessageWindow.patch
> 
> Like FX I also rename the XP image to -XP and remove the -aero from the aero
> image.

Oops, wrong bug. This one has no images to rename.

Comment 3

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 8616373 [details] [diff] [review]
unifyMessageWindow.patch

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

Hmm.  I've got a lot of questions, so I think I'm going to have to say "r-", at least until they're answered.  ;)

::: mail/themes/windows/mail/messageWindow.css
@@ +20,5 @@
>  #mail-toolbox:-moz-lwtheme {
>    text-shadow: none;
>  }
>  
> +@media (-moz-os-version: windows-xp) {

Why are we putting these in a windows-xp block?  It looks like they used to apply to the not-windows-xp code, too…

@@ +21,5 @@
>    text-shadow: none;
>  }
>  
> +@media (-moz-os-version: windows-xp) {
> +  #messagepanebox {

I don't think we need this rule here, because "text-shadow: none" is in the main #messagepanebox rule above.

@@ +71,5 @@
> +    border-bottom: 4px solid highlight;
> +  }
> +}
> +
> +@media (-moz-windows-compositor) {

Shouldn't this also be in the "@media not all and (-moz-os-version: windows-xp) {" section?

Comment 4

[Richard Marti (:Paenglab)]

Attachment: unifyMessageWindow.patch v2

(In reply to Blake Winton (:bwinton) from comment #3)
> Comment on attachment 8616373 [details] [diff] [review]
> unifyMessageWindow.patch
> 
> Review of attachment 8616373 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hmm.  I've got a lot of questions, so I think I'm going to have to say "r-",
> at least until they're answered.  ;)
> 
> ::: mail/themes/windows/mail/messageWindow.css
> @@ +20,5 @@
> >  #mail-toolbox:-moz-lwtheme {
> >    text-shadow: none;
> >  }
> >  
> > +@media (-moz-os-version: windows-xp) {
> 
> Why are we putting these in a windows-xp block?  It looks like they used to
> apply to the not-windows-xp code, too…

This two rules are XP only.

> @@ +21,5 @@
> >    text-shadow: none;
> >  }
> >  
> > +@media (-moz-os-version: windows-xp) {
> > +  #messagepanebox {
> 
> I don't think we need this rule here, because "text-shadow: none" is in the
> main #messagepanebox rule above.

Correct -> removed.

> @@ +71,5 @@
> > +    border-bottom: 4px solid highlight;
> > +  }
> > +}
> > +
> > +@media (-moz-windows-compositor) {
> 
> Shouldn't this also be in the "@media not all and (-moz-os-version:
> windows-xp) {" section?

XP doesn't use -moz-windows-compositor and thus doesn't use the rules in this query. (see also https://dxr.mozilla.org/mozilla-central/source/browser/themes/windows/devedition.css#28 and the lines 14 and 15 where it's used too in FX).

If you like I can move it inside the not XP section to be 200% sure.

Comment 5

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 8622535 [details] [diff] [review]
unifyMessageWindow.patch v2

Looks much better, thanks!  :)

Comment 6

[Magnus Melin [:mkmelin]]

https://hg.mozilla.org/comm-central/rev/a0eeb9e33a65 -> FIXED