1519519 - (css-min-max) [css-values] support CSS min() / max() / clamp() functions.

Status: RESOLVED FIXED

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

Description

In https://bugzilla.mozilla.org/show_bug.cgi?id=1462233 env() was implemented, resulting in 0px.

margin-left: margin: max(2em, env(safe-area-inset-left));

This should result in 2em, but results in 0px in Gecko. This makes sites that use this feature have the wrong margin/padding in Gecko on all platforms.

margin-left: foo(bar, env(baz));

This should fail to parse (since foo(), bar, and baz are not supported), but also results in 0px in Gecko. This is bogus and may make it hard to introduce new things if content starts relying on bogus values meaning 0px in Gecko (e.g. using CSS.supports('margin: foo(bar, env(baz))') to detect Gecko).

Comment 1

[Emilio Cobos Álvarez (:emilio)]

(In reply to Simon Pieters from comment #0)

In https://bugzilla.mozilla.org/show_bug.cgi?id=1462233 env() was implemented, resulting in 0px.

margin-left: margin: max(2em, env(safe-area-inset-left));

This should result in 2em, but results in 0px in Gecko. This makes sites that use this feature have the wrong margin/padding in Gecko on all platforms.

Well, we don't support max(), so it's not a surprise. It just becomes invalid at computed value time.

I thought we had a bug on file for them but I cannot find it, so if somebody dupes it I'd appreciate it.

margin-left: foo(bar, env(baz));

This should fail to parse (since foo(), bar, and baz are not supported), but also results in 0px in Gecko. This is bogus and may make it hard to introduce new things if content starts relying on bogus values meaning 0px in Gecko (e.g. using CSS.supports('margin: foo(bar, env(baz))') to detect Gecko).

This is wrong. This should parse given how env() is spec'd. It has a valid env() function, so it's a token stream the same way as if it had var(--foo). This looks like a bug in WebKit / Blink. It looks they're bogus also with custom properties.

CSS.supports("margin: var(--baz) 3209")

returns true as expected, but.

CSS.supports("margin: foobar(var(--baz)) 3209")

returns false, even though it definitely should parse. Of course it does not parse in Blink, but that looks like a bug. Or am I missing something?

Comment 2

[Emilio Cobos Álvarez (:emilio)]

So the upshot is that to properly test for this you need to test for "margin: max(0px, 0px)", for example, not anything containing "env()"

Comment 3

[Gérard Talbot]

http://www.gtalbot.org/BrowserBugsSection/CSS3Values/trac-webkit-simple-minmax.html

Epiphany Technology Preview 3.31.3-71-g3b33bb6ea (WebKitGTK+ 2.23.1) pass that test.

Chromium 73.0.3670.0 and Firefox 66.0a1 buildID=20190112094119 fail that test.

Comment 4

[Simon Pieters [:zcorpan]]

I read the spec for env() now and indeed it will use the initial value if it's invalid at computed value time, which is pretty unintuitive. (I would have thought that if max() is not supported, it would be dropped during the cascade.)

So yeah, I think what remains is to implement min() and max().

Comment 5

[Xidorn Quan [:xidorn] UTC+10]

Implementing min() / max() can be a non-trivial amount of work, mostly because it needs to propagate into layout for many cases given that it supports percentage. Probably we should first have more computed value be using Rust types (which emilio is doing?), ideally we should remove nsStyleCoord I guess...

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Agreed.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

I worked a bit today on the blockers for this. After those land, I think the only big piece of work is gradients (which may be pretty annoying actually). Other than that the other <length-percentage> properties should be fairly straight-forward.

Comment 8

[Xiaocheng Hu]

Hi,

This is Xiaocheng from the Chrome team.

Now that both Safari and Chrome have shipped these functions, and there's already a WPT test suite, does Firefox has plan to implement them?

Thanks!

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Yes, it's on the backlog of stuff to do and I'm doing some dependent work as I find time for that (see the "Depends on:" bugs above).

Comment 10

[Xiaocheng Hu]

Great to know that, thanks!

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1519519 - Enable min() / max() / clamp() support by default. r=#style,heycam

Will send an intent. The only issues I'm aware of are serialization /
simplification ones where every browser does its own thing already, and we're
the one that behaves the closest to the spec.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/47d7f75d469f
Enable min() / max() / clamp() support by default. r=heycam

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a63cf4d3ecd7
Fix some tests that were testing as invalid now-valid syntax.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4c595c47d6b6
Annotate some more now-passing tests.

Comment 15

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/47d7f75d469f
https://hg.mozilla.org/mozilla-central/rev/a63cf4d3ecd7
https://hg.mozilla.org/mozilla-central/rev/4c595c47d6b6