Closed Bug 1313830 Opened 4 years ago Closed 4 years ago

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

Categories

(Firefox :: Theme, defect, P3)

defect

Tracking

()

RESOLVED FIXED
Firefox 53
Tracking Status
firefox53 --- fixed

People

(Reporter: dao, Assigned: katika2987, Mentored)

References

Details

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

Attachments

(1 file, 2 obsolete files)

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
Kate is going to take a look at this one.
Assignee: nobody → katika2987
Attached patch not-at-all-replacement.patch (obsolete) — Splinter Review
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!
Attachment #8805760 - Attachment is obsolete: true
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)
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)
Comment on attachment 8805787 [details] [diff] [review]
Bug-1313830.patch

Looks good. Thanks!
Attachment #8805787 - Flags: review?(dao+bmo) → review+
Oops, looks like we never landed this. Next time you can just add the checkin-needed keyword after getting review granted on your patch.
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
https://hg.mozilla.org/mozilla-central/rev/6ec29f454b89
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Blocks: 1411210
You need to log in before you can comment on or make changes to this bug.