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

Status: RESOLVED FIXED

(Firefox :: Theme, defect, P3)

Description

[Dão Gottwald [:dao]]

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

Comment 1

[Dão Gottwald [:dao]]

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

Comment 2

[Michael Henretty [:mikehenrty][:mhenretty]]

Kate is going to take a look at this one.

Comment 3

[katecastellano]

Attachment: not-at-all-replacement.patch

Comment 4

[Dão Gottwald [:dao]]

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a3fe39bb76a8e53c36c1a3f8f0f8b1de8591ba89

Comment 5

[Dão Gottwald [:dao]]

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!

Comment 6

[katecastellano]

Attachment: not-at-all-replacement-no-superfluos-spaces.patch

Comment 7

[katecastellano]

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

tests passed and I updated your changes

Comment 8

[katecastellano]

Attachment: Bug-1313830.patch

Fixed broken patch file and added previous said fixes

Comment 9

[Dão Gottwald [:dao]]

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

Looks good. Thanks!

Comment 10

[Dão Gottwald [:dao]]

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

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6ec29f454b89
fixed media queries to not use 'not at all', r=dao

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6ec29f454b89