1396057 - stylo: Bring back calc support inside media queries

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement, P4)

Description

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

This was implemented in stylo but not in gecko. We had to temporarily disable this just in case in Bug 1390339. We should bring this support back and ship that in the next release. See Bug 1390339 comment #10.

It's not blocking stylo.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

I was dealing with bug 1408308 and it'd be nice to update media query parsing to the spec.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1396057: Fix indentation of the media query code.

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

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1396057: Fix evaluation of rem inside media queries.

Also fix the tests so that they would've caught this.

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

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1396057: Support calc() in media queries.

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

Comment 5

[Emilio Cobos Álvarez (:emilio)]

(This is just the first step in updating to the spec, we should add support for proper expressions in another bug)

Comment 6

[Emilio Cobos Álvarez (:emilio)]

This needs the following expectations update:

  https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c2b7db9bd35452ead7acc1dfc3aa0c1f9cfeb8e

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8927356 [details]
Bug 1396057: Fix indentation of the media query code.

Xidorn and Cam are on PTO. Manish, could you review this?

Comment 8

[Manish Goregaokar [:manishearth]]

Comment on attachment 8927356 [details]
Bug 1396057: Fix indentation of the media query code.

https://reviewboard.mozilla.org/r/198664/#review203960

I think this style change is very subjective, but whatever. The new style seems to be what rustfmt does so that seems ok.

Comment 9

[Manish Goregaokar [:manishearth]]

Comment on attachment 8927357 [details]
Bug 1396057: Fix evaluation of rem inside media queries.

https://reviewboard.mozilla.org/r/198666/#review203962

Comment 10

[Manish Goregaokar [:manishearth]]

Comment on attachment 8927358 [details]
Bug 1396057: Support calc() in media queries.

https://reviewboard.mozilla.org/r/198668/#review203964

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6bed71583aa9
Change tests of calc in media-queries so that the root element style has been resolved first. r=Manishearth
https://hg.mozilla.org/integration/autoland/rev/47752e9824da
Update test expectations. r=me

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Servo bits @ https://hg.mozilla.org/integration/autoland/rev/b7ebbb451e5a

Comment 13

[Ryan Jones-Ward [:sciguyryan]]

Does the web developer documentation need to be updated to account for these changes by any chance?

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/6bed71583aa9
https://hg.mozilla.org/mozilla-central/rev/47752e9824da

Comment 17

[Chris Mills [:cmills]]

(In reply to Ryan Jones [:sciguyryan] from comment #13)
> Does the web developer documentation need to be updated to account for these
> changes by any chance?

Probably, yes.

Comment 18

[Chris Mills [:cmills]]

Documented:

* Adding in note about it to browser compat data: https://github.com/mdn/browser-compat-data/pull/886
* Note in Fx59 rel notes: https://developer.mozilla.org/en-US/Firefox/Releases/59#CSS

Let me know if this looks OK. Thanks!

Comment 19

[Emilio Cobos Álvarez (:emilio)]

I don't think it's worth mentioning that calc() in media queries was disabled on stylo, since stylo was never shipped before 57, and Gecko never supported it.

Other than that it looks good!

Comment 20

[Chris Mills [:cmills]]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #19)
> I don't think it's worth mentioning that calc() in media queries was
> disabled on stylo, since stylo was never shipped before 57, and Gecko never
> supported it.
> 
> Other than that it looks good!

OK; I've simplified this note to just say it was adding in 59. The calc() compat data contains the exact info.