Closed Bug 1519519 (css-min-max) Opened 2 years ago Closed 1 year ago

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

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla75
Tracking Status
firefox75 --- fixed

People

(Reporter: zcorpan, Assigned: emilio)

References

()

Details

(Keywords: testcase, Whiteboard: [layout:backlog:2020q1])

Attachments

(1 file)

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

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

Flags: needinfo?(zcorpan)
Summary: Properly support CSS max() and env() → [css-values] support CSS min() / max() functions.

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

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.

Keywords: testcase
OS: Unspecified → All
Hardware: Unspecified → All
Version: unspecified → Trunk
Priority: -- → P3

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

Flags: needinfo?(zcorpan)

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

Agreed.

Depends on: 1527410
Depends on: 1527438
Depends on: 1527542

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.

Whiteboard: [layout:backlog]
Depends on: 1559545
Depends on: 1561738
Depends on: 1604076
Alias: css-min-max
Summary: [css-values] support CSS min() / max() functions. → [css-values] support CSS min() / max() / clamp() functions.
Depends on: 1604160
Depends on: 1607049
Depends on: 1609256
Depends on: 1609428
Depends on: 1609711
Depends on: 1609737
Whiteboard: [layout:backlog] → [layout:backlog:2020]
Depends on: 1610801

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!

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

Great to know that, thanks!

Depends on: 1613010
Depends on: 1613491
Depends on: 1616691

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.

Assignee: nobody → emilio
Status: NEW → ASSIGNED
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/47d7f75d469f
Enable min() / max() / clamp() support by default. r=heycam
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/a63cf4d3ecd7
Fix some tests that were testing as invalid now-valid syntax.
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/4c595c47d6b6
Annotate some more now-passing tests.
Whiteboard: [layout:backlog:2020] → [layout:backlog:2020q1]
See Also: → 1432452
Duplicate of this bug: 1632361
You need to log in before you can comment on or make changes to this bug.