Replace @media not all and (-moz-...) with @media (-moz-...: 0)

RESOLVED FIXED in Firefox 59

Status

()

P3
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: dao, Assigned: vashishthmayur, Mentored)

Tracking

({good-first-bug})

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

Firefox Tracking Flags

(firefox58 wontfix, firefox59 fixed)

Details

(Whiteboard: [lang=css])

Attachments

(4 attachments)

(Reporter)

Description

a year ago
+++ This bug was initially created as a clone of Bug #1313830 +++

We currently use "@media not all and (-moz-...)" with -moz-mac-yosemite-theme, -moz-windows-classic, and -moz-touch-enabled:

http://searchfox.org/mozilla-central/search?q=%40media+not+all+and+(-moz&case=true&regexp=false&path=

These can be simplified to "@media (-moz-...: 0)" like here:

http://searchfox.org/mozilla-central/rev/d30462037ffea383e74c42542c820cf65b2b144e/browser/themes/windows/browser.css#515
Hi, I would like to work on this as my beginning bugs.
Dão: Any pointers as to where I should start?
I have already set up the dev environment.
Flags: needinfo?(dao+bmo)
(Reporter)

Comment 2

a year ago
(In reply to Manish Kumar from comment #1)
> Hi, I would like to work on this as my beginning bugs.
> Dão: Any pointers as to where I should start?

Hi, start with the first link from comment 0. It shows you what files and lines you need to change.
Flags: needinfo?(dao+bmo)
Dão: Thanks for this info.

I would like to work on this. 
can you assign this bug to me?
(Reporter)

Comment 4

a year ago
Sure :)
Assignee: nobody → 1991manish.kumar
(Reporter)

Comment 5

a year ago
Are you still going to work on this? Let me know if you need help.
Flags: needinfo?(1991manish.kumar)
(In reply to Dão Gottwald [::dao] from comment #5)
> Are you still going to work on this? Let me know if you need help.

Thanks Dão, I will work on this bug, actually I was working on 1 more bug earlier and some unexpected error is coming in that. Once I am done with that bug, I will work on this.
Flags: needinfo?(1991manish.kumar)
(Reporter)

Comment 7

a year ago
Any update?
Flags: needinfo?(1991manish.kumar)
(Assignee)

Comment 8

a year ago
I would like to work on this bug. Please assign it to me.
(Reporter)

Comment 9

a year ago
Okay.
Assignee: 1991manish.kumar → vashishthmayur
Flags: needinfo?(1991manish.kumar)
(Assignee)

Comment 10

a year ago
1 more thing. I would like to know where is the source code repository. Couldn't find it. Sorry for such rookie question.
(Reporter)

Comment 11

a year ago
(In reply to Mayur Vashishth from comment #10)
> 1 more thing. I would like to know where is the source code repository.
> Couldn't find it. Sorry for such rookie question.

It's at <https://hg.mozilla.org/mozilla-central>. See also <https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Source_Code/Mercurial>.

Comment 12

a year ago
I have started working on this bug, but I am not sure if you I need to replace the ones that are missing the "not all" part. You have specified only the lines that that are "@media not all and...., but there are some that I found with the same parameters you need changed that are lacking the "not all part". I will assume they need to be changed as well, if not I will fix it.
Flags: needinfo?(vashishthmayur)
Comment hidden (mozreview-request)
(Reporter)

Comment 14

a year ago
(In reply to Kristiyan from comment #12)
> I have started working on this bug, but I am not sure if you I need to
> replace the ones that are missing the "not all" part. You have specified
> only the lines that that are "@media not all and...., but there are some
> that I found with the same parameters you need changed that are lacking the
> "not all part". I will assume they need to be changed as well, if not I will
> fix it.

Only those with "not all and" need to be replaced. And the queried feature shouldn't change, e.g. "@media not all and (-moz-mac-yosemite-theme)" should become "@media (-moz-mac-yosemite-theme: 0)" rather than "@media (-moz-windows-default-theme: 0)".
Flags: needinfo?(vashishthmayur)

Comment 15

a year ago
Sorry for the misunderstanding, I will redo it in the correct way. Thank you.
Comment hidden (mozreview-request)

Comment 17

a year ago
Created attachment 8935839 [details]
GREP- left occurrences of "not all and" in browser/themes and toolkit/themes
(Reporter)

Comment 18

a year ago
The patch fails to apply in some of the patched files. Are you working on mozilla-central? Is your copy up to date?

Comment 19

a year ago
I reinstalled my VM yesterday evening and updated it as well, should be. Should I update it again and recommit?
(Reporter)

Comment 20

a year ago
(In reply to Kristiyan from comment #19)
> I reinstalled my VM yesterday evening and updated it as well, should be.
> Should I update it again and recommit?

Yeah, please do. Thanks!
Comment hidden (mozreview-request)

Comment 22

a year ago
Created attachment 8936481 [details]
media_test.txt

Updated my build, re-did the change set and GREPed for any leftovers in ../toolkit/themes and ../browser/themes.
Attachment #8936481 - Flags: feedback+
(Reporter)

Comment 23

a year ago
mozreview-review
Comment on attachment 8935600 [details]
[mq]: Bug 1411210 - Update / Replace @media not all and (-moz-...)

https://reviewboard.mozilla.org/r/206482/#review213088

::: toolkit/themes/mobile/mozapps/plugins/pluginProblem.css:93
(Diff revision 3)
>  .msgTapToPlay,
>  .msgClickToPlay {
>    text-decoration: underline;
>  }
>  
> -@media not all and (-moz-touch-enabled) {
> +@media (-moz-touch-enabled) {

This should be @media (-moz-touch-enabled: 0) {

Looks good otherwise!
Comment hidden (mozreview-request)

Comment 25

a year ago
Fixed that, sorry I missed it, thank you for the support you've been very helpful.
(Reporter)

Updated

a year ago
Attachment #8935600 - Flags: review+
(Reporter)

Comment 26

a year ago
Comment on attachment 8936581 [details]
[mq]: Bug 1411210 - Final Touches / Replace @media not all and (-moz-...)

Thanks! Next time please fold modifications into your original patch so that we have one patch to land rather than two.
Attachment #8936581 - Flags: review+

Comment 27

a year ago
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ae78c4fe253a
Replace @media not all and (-moz-...) with @media (-moz-...: 0). r=dao

Comment 28

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ae78c4fe253a
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Too late for Beta58. Mark 58 won't fix.
status-firefox58: affected → wontfix
You need to log in before you can comment on or make changes to this bug.