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

Status: RESOLVED FIXED

(Firefox :: Theme, enhancement, P3)

Description

[Dão Gottwald [:dao]]

+++ 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

Comment 1

[Manish [:manishkk]]

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.

Comment 2

[Dão Gottwald [:dao]]

(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.

Comment 3

[Manish [:manishkk]]

Dão: Thanks for this info.

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

Comment 4

[Dão Gottwald [:dao]]

Sure :)

Comment 5

[Dão Gottwald [:dao]]

Are you still going to work on this? Let me know if you need help.

Comment 6

[Manish [:manishkk]]

(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.

Comment 7

[Dão Gottwald [:dao]]

Any update?

Comment 8

[Mayur Vashishth]

I would like to work on this bug. Please assign it to me.

Comment 9

[Dão Gottwald [:dao]]

Okay.

Comment 10

[Mayur Vashishth]

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

Comment 11

[Dão Gottwald [:dao]]

(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

[Kristiyan]

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.

Comment 13

[Kristiyan]

Attachment: [mq]: Bug 1411210 - Update / Replace @media not all and (-moz-...)

Review commit: https://reviewboard.mozilla.org/r/206482/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/206482/

Comment 14

[Dão Gottwald [:dao]]

(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)".

Comment 15

[Kristiyan]

Sorry for the misunderstanding, I will redo it in the correct way. Thank you.

Comment 16

[Kristiyan]

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/206482/diff/1-2/

Comment 17

[Kristiyan]

Attachment: GREP- left occurrences of "not all and" in browser/themes and toolkit/themes

Comment 18

[Dão Gottwald [:dao]]

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

[Kristiyan]

I reinstalled my VM yesterday evening and updated it as well, should be. Should I update it again and recommit?

Comment 20

[Dão Gottwald [:dao]]

(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 21

[Kristiyan]

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/206482/diff/2-3/

Comment 22

[Kristiyan]

Attachment: media_test.txt

Updated my build, re-did the change set and GREPed for any leftovers in ../toolkit/themes and ../browser/themes.

Comment 23

[Dão Gottwald [:dao]]

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 24

[Kristiyan]

Attachment: [mq]: Bug 1411210 - Final Touches / Replace @media not all and (-moz-...)

Review commit: https://reviewboard.mozilla.org/r/207342/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/207342/

Comment 25

[Kristiyan]

Fixed that, sorry I missed it, thank you for the support you've been very helpful.

Comment 26

[Dão Gottwald [:dao]]

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.

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ae78c4fe253a
Replace @media not all and (-moz-...) with @media (-moz-...: 0). r=dao

Comment 28

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/ae78c4fe253a

Comment 29

[Gerry Chang [:gchang]]

Too late for Beta58. Mark 58 won't fix.