The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in Firefox 12

Status

()

Firefox
Theme
--
trivial
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dao, Assigned: jyeo)

Tracking

Trunk
Firefox 12
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor=dao][lang=css])

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

5 years ago
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
(Reporter)

Updated

5 years ago
Whiteboard: [good first bug][mentor=dao][lang=css]
(Reporter)

Updated

5 years ago
Summary: Utilize @-rule nesting → Replace :-moz-system-metric with nested @media blocks
(Assignee)

Comment 1

5 years ago
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
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
Attachment #582757 - Flags: review?(dao)
(Reporter)

Comment 2

5 years ago
(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.
(Reporter)

Comment 3

5 years ago
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
(Assignee)

Comment 4

5 years ago
Created attachment 583530 [details] [diff] [review]
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!
Attachment #582757 - Attachment is obsolete: true
Attachment #582757 - Flags: review?(dao)
Attachment #583530 - Flags: review?(dao)
(Reporter)

Comment 5

5 years ago
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 :(
Attachment #583530 - Flags: review?(dao) → review-
(Assignee)

Comment 6

5 years ago
Created attachment 583830 [details]
709578v3.patch
(Assignee)

Comment 7

5 years ago
Created attachment 583831 [details] [diff] [review]
709578v4.patch
Assignee: nobody → jasonyeo88
Attachment #583530 - Attachment is obsolete: true
Attachment #583830 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #583831 - Flags: review?(dao)
(Reporter)

Comment 8

5 years ago
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.
Attachment #583831 - Flags: review?(dao) → review+
(Assignee)

Comment 9

5 years ago
Created attachment 584455 [details] [diff] [review]
709578v5.patch

okay I have made the changes. So should I flag this as checkin-needed?
Attachment #583831 - Attachment is obsolete: true
Attachment #584455 - Flags: review+
(Reporter)

Comment 10

5 years ago
Yep. Thanks!
Keywords: checkin-needed
(Reporter)

Comment 11

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/3e315088e243
Keywords: checkin-needed
Summary: Replace :-moz-system-metric with nested @media blocks → Replace :-moz-system-metric with @media blocks
Target Milestone: --- → Firefox 12
Fixed for Firefox 12.  Thanks for the patch!
https://hg.mozilla.org/mozilla-central/rev/3e315088e243
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.