709578 - Replace :-moz-system-metric with @media blocks

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Dão Gottwald [:dao]]

In particular here:
http://mxr.mozilla.org/mozilla-central/source/browser/themes/winstripe/browser-aero.css#297

and here:
http://mxr.mozilla.org/mozilla-central/source/browser/themes/winstripe/places/places-aero.css

Comment 1

[Jason Yeo[:jyeo]]

Attachment: 709578.patch

Hi,

I'm new to moz-central code base, so this is my first patch. I'm not sure if I'm doing the right thing here. Please let me know and I will make changes to it ASAP.

There are some other css files where the -moz-system-metric(windows-default-theme) also appears. Should I change that too? They appear in:
http://mxr.mozilla.org/mozilla-central/source/layout/reftests/editor/xul/input.css#15
http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/winstripe/global/textbox-aero.css
http://mxr.mozilla.org/mozilla-central/source/browser/themes/winstripe/browser.css#1554

Comment 2

[Dão Gottwald [:dao]]

(In reply to Jason Yeo(:jyeo) from comment #1)
> Created attachment 582757 [details] [diff] [review]
> 709578.patch
> 
> Hi,
> 
> I'm new to moz-central code base, so this is my first patch. I'm not sure if
> I'm doing the right thing here. Please let me know and I will make changes
> to it ASAP.
> 
> There are some other css files where the
> -moz-system-metric(windows-default-theme) also appears. Should I change that
> too? They appear in:
> http://mxr.mozilla.org/mozilla-central/source/layout/reftests/editor/xul/
> input.css#15

We don't ship this code, it's only for tests, so it's probably not worth it.

> http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/winstripe/
> global/textbox-aero.css
> http://mxr.mozilla.org/mozilla-central/source/browser/themes/winstripe/
> browser.css#1554

Yeah, these would be fine to be changed as well.

Comment 3

[Dão Gottwald [:dao]]

Comment on attachment 582757 [details] [diff] [review]
709578.patch

>--- a/browser/themes/winstripe/places/places-aero.css	Sun Dec 18 18:50:13 2011 -0800
>+++ b/browser/themes/winstripe/places/places-aero.css	Mon Dec 19 15:51:27 2011 +0800

>+@media -moz-windows-default-theme {
>+  #bookmarksPanel, #history-panel {

nit: please always break the line after the comma

Comment 4

[Jason Yeo[:jyeo]]

Attachment: 709578v2.patch

Hi,

I have added a newline after the comma for browser/themes/winstripe/places/places-aero.css.

Also, I have edited 
http://mxr.mozilla.org/mozilla-central/source/browser/themes/winstripe/browser.css#1554
and
http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/winstripe/global/textbox-aero.css

Let me know if I'm doing the right thing. Thanks!

Comment 5

[Dão Gottwald [:dao]]

Comment on attachment 583530 [details] [diff] [review]
709578v2.patch

I wish this worked, but apparently you need to write @media (-moz-windows-default-theme) instead of @media -moz-windows-default-theme :(

Comment 6

[Jason Yeo[:jyeo]]

Attachment: 709578v3.patch

Comment 7

[Jason Yeo[:jyeo]]

Attachment: 709578v4.patch

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 583831 [details] [diff] [review]
709578v4.patch

>diff -r d75ebb37080e browser/themes/winstripe/browser-aero.css
>--- a/browser/themes/winstripe/browser-aero.css	Sun Dec 18 18:50:13 2011 -0800
>+++ b/browser/themes/winstripe/browser-aero.css	Thu Dec 22 01:30:41 2011 +0800

>-  #toolbar-menubar[autohide=true]:-moz-system-metric(windows-default-theme) {
>-    background-color: transparent;

>+    #toolbar-menubar[autohide=true] {
>+      background-color: transparent;
>+    }

You need to add !important here, as this selector now loses against others.

Other than that this patch is fine.

Comment 9

[Jason Yeo[:jyeo]]

Attachment: 709578v5.patch

okay I have made the changes. So should I flag this as checkin-needed?

Comment 10

[Dão Gottwald [:dao]]

Yep. Thanks!

Comment 11

[Dão Gottwald [:dao]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/3e315088e243

Comment 12

[Matt Brubeck (:mbrubeck)]

Fixed for Firefox 12.  Thanks for the patch!
https://hg.mozilla.org/mozilla-central/rev/3e315088e243