The default bug view has changed. See this FAQ.

Replace @media not all and (-moz-windows-default-theme) with @media (-moz-windows-default-theme: 0)

RESOLVED FIXED in Firefox 53

Status

()

Firefox
Theme
P3
normal
RESOLVED FIXED
5 months ago
3 months ago

People

(Reporter: dao, Assigned: katecastellano, Mentored)

Tracking

({good-first-bug})

Trunk
Firefox 53
good-first-bug
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

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

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

5 months ago
We use "@media not all and (-moz-windows-default-theme)" in a couple of places:

https://dxr.mozilla.org/mozilla-central/search?q=not+all+and+(-moz-windows-default-theme)&redirect=false

and it can be simplified to "@media (-moz-windows-default-theme: 0)" like here:

https://dxr.mozilla.org/mozilla-central/rev/1561c917ee27c3ea04bd69467e5b8c7c08102f2a/browser/themes/shared/icon-colors.inc.svg#16
(Reporter)

Comment 1

5 months ago
While we're at it, we could do the same for -moz-windows-compositor and -moz-windows-classic:

https://dxr.mozilla.org/mozilla-central/search?q=not+all+and+(-moz-windows-compositor)&redirect=true

https://dxr.mozilla.org/mozilla-central/search?q=not+all+and+(-moz-windows-classic)&redirect=true
Kate is going to take a look at this one.
Assignee: nobody → katika2987
(Assignee)

Comment 3

5 months ago
Created attachment 8805760 [details] [diff] [review]
not-at-all-replacement.patch
(Reporter)

Comment 4

5 months ago
Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a3fe39bb76a8e53c36c1a3f8f0f8b1de8591ba89
(Reporter)

Comment 5

5 months ago
Comment on attachment 8805760 [details] [diff] [review]
not-at-all-replacement.patch

Some feedback while we're waiting for results from the Try server run:

>--- a/browser/themes/windows/browser.css
>+++ b/browser/themes/windows/browser.css
>@@ -187,8 +187,8 @@ toolbar:-moz-lwtheme {
>   transition: min-height 170ms ease-out, max-height 170ms ease-out, visibility 170ms linear;
> }
> 
>-@media not all and (-moz-windows-compositor),
>-       not all and (-moz-windows-default-theme) {
>+@media (-moz-windows-compositor : 0),

nit: there's an extra space before : that shouldn't be there

> @media not all and (-moz-os-version: windows-vista) and (-moz-windows-default-theme) {
>-  @media not all and (-moz-os-version: windows-win7) and (-moz-windows-default-theme) {
>+  @media not all and  (-moz-os-version: windows-win7) and (-moz-windows-default-theme) {

nit: you've added a superfluous space here

>--- a/browser/themes/windows/devedition.css
>+++ b/browser/themes/windows/devedition.css
>@@ -191,8 +191,8 @@
>   }
> 
>   /* Use proper menu text styling in Win7 classic mode (copied from browser.css) */
>-  @media not all and (-moz-windows-compositor),
>-         not all and (-moz-windows-default-theme) {
>+  @media (-moz-windows-compositor : 0),

superfluous space before : again

>--- a/devtools/client/webconsole/test/test-file-location.js
>+++ b/devtools/client/webconsole/test/test-file-location.js
>@@ -3,6 +3,7 @@
> /* Any copyright is dedicated to the Public Domain.
>  * http://creativecommons.org/publicdomain/zero/1.0/ */
> 
>+
> console.log("message for level log");
> console.info("message for level info");
> console.warn("message for level warn");

Unrelated change, please revert this

Looks good otherwise!
(Assignee)

Comment 6

5 months ago
Created attachment 8805763 [details] [diff] [review]
not-at-all-replacement-no-superfluos-spaces.patch
(Assignee)

Updated

5 months ago
Attachment #8805760 - Attachment is obsolete: true
(Assignee)

Comment 7

5 months ago
Comment on attachment 8805763 [details] [diff] [review]
not-at-all-replacement-no-superfluos-spaces.patch

tests passed and I updated your changes
Attachment #8805763 - Flags: review?(dao+bmo)
(Assignee)

Comment 8

5 months ago
Created attachment 8805787 [details] [diff] [review]
Bug-1313830.patch

Fixed broken patch file and added previous said fixes
Attachment #8805763 - Attachment is obsolete: true
Attachment #8805763 - Flags: review?(dao+bmo)
Attachment #8805787 - Flags: review?(dao+bmo)
(Reporter)

Comment 9

5 months ago
Comment on attachment 8805787 [details] [diff] [review]
Bug-1313830.patch

Looks good. Thanks!
Attachment #8805787 - Flags: review?(dao+bmo) → review+
(Reporter)

Comment 10

3 months ago
Oops, looks like we never landed this. Next time you can just add the checkin-needed keyword after getting review granted on your patch.

Comment 11

3 months ago
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6ec29f454b89
fixed media queries to not use 'not at all', r=dao
(Reporter)

Updated

3 months ago
status-firefox52: affected → ---

Comment 12

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6ec29f454b89
Status: NEW → RESOLVED
Last Resolved: 3 months ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
You need to log in before you can comment on or make changes to this bug.