1764850 - Support CSS round()

Status: RESOLVED FIXED

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

Description

[Yves Goergen [:ygoe]]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:99.0) Gecko/20100101 Firefox/99.0

Steps to reproduce:

The CSS round() function would be extremely helpful in avoiding sub-pixel layout mess in text paragraphs when defining line heights in % or similar situations.

Here's the spec: https://www.w3.org/TR/css-values-4/#round-func

I tried to find an existing bug here but the simple search found millions of substrings for "round" and the advanced search found nothing whatever I did. Please feel free to close as duplicate if one exists, the bug search is not working for me (might need fixing).

Comment 1

[BugBot [:suhaib / :marco]]

The Bugbug bot thinks this bug should belong to the 'Firefox::Address Bar' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

This should be relatively straight forward to implement following the Min/Max/Clamp code in here: https://searchfox.org/mozilla-central/rev/d34f9713ae128a3138c2b70d8041a535f1049d19/servo/components/style/values/generics/calc.rs#84

Comment 3

[Yves Goergen [:ygoe]]

I don't understand much of that code, but one specialty of the round() function is that it can mix units (of a type, e.g. length), for example em with px, or % with em:

div
{
    height: round(2em, 1px);
}

Here's the use case I'm most interested in. It's about setting font sizes and (more importantly) line heights to full pixels from abstract variables that can easily be changed per page:

p
{
    --default-font-size: 16px;
    --default-line-height-factor: 1.4;
    /* would result in 22.4px which round irregularly over multiple lines */

    font-size: var(--default-font-size);
    line-height: round(var(--default-font-size) * var(--default-line-height-factor), 1px)
    /* rather fix that to 22px for even line spacing */
}

Comment 4

[Yves Goergen [:ygoe]]

Reference to the Chrome bug covering all math functions: https://crbug.com/1284199

Comment 5

[Emilio Cobos Álvarez [:emilio]]

(In reply to Yves Goergen from comment #3)

I don't understand much of that code, but one specialty of the round() function is that it can mix units (of a type, e.g. length), for example em with px, or % with em

Yeah, that should just work and is how min() / max() / clamp() works. When they get computed you get pixels, and you might need to preserve them until used value time where you get percentages.

It isn't a one-liner but it should be a matter of extending the min() / max() / clamp() code.

Comment 6

[Connor Pearson]

Attachment: Bug 1764850 Implement CSS round() function. r=emilio

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c7e10bd5cd69
Implement CSS round() function. r=emilio

Comment 8

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/36825 for changes under testing/web-platform/tests

Comment 9

[Natalia Csoregi [:nataliaCs]]

Backed out for bustage on nsStyleStruct.cpp

• backout: https://hg.mozilla.org/integration/autoland/rev/a7488b36c7d6c897fcac1d818faf088552de5c10
• push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=b2GdmoKzQs6zaKwHRGSu3w.0&revision=c7e10bd5cd69d84bf2f9ed6faef201ea5bfcb949
• failure log: https://treeherder.mozilla.org/logviewer?job_id=395603030&repo=autoland&lineNumber=34473

[task 2022-11-04T21:36:07.039Z] 21:36:07     INFO -  In file included from Unified_cpp_layout_style4.cpp:128:0:
[task 2022-11-04T21:36:07.039Z] 21:36:07     INFO -  /builds/worker/checkouts/gecko/layout/style/nsStyleStruct.cpp: In lambda function:
[task 2022-11-04T21:36:07.040Z] 21:36:07    ERROR -  /builds/worker/checkouts/gecko/layout/style/nsStyleStruct.cpp:3628:7: error: control reaches end of non-void function [-Werror=return-type]
[task 2022-11-04T21:36:07.040Z] 21:36:07     INFO -         }();
[task 2022-11-04T21:36:07.041Z] 21:36:07     INFO -         ^
[task 2022-11-04T21:36:07.041Z] 21:36:07     INFO -  /builds/worker/checkouts/gecko/layout/style/nsStyleStruct.cpp: In lambda function:
[task 2022-11-04T21:36:07.042Z] 21:36:07    ERROR -  /builds/worker/checkouts/gecko/layout/style/nsStyleStruct.cpp:3628:7: error: control reaches end of non-void function [-Werror=return-type]
[task 2022-11-04T21:36:07.042Z] 21:36:07     INFO -         }();
[task 2022-11-04T21:36:07.043Z] 21:36:07     INFO -         ^
[task 2022-11-04T21:36:07.043Z] 21:36:07     INFO -  cc1plus: all warnings being treated as errors

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/031200e2a50d
Implement CSS round() function. r=emilio

Comment 11

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/031200e2a50d

Comment 12

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 13

[Hamish Willee]

FF108 MDN docs work for this can be tracked in https://github.com/mdn/content/issues/22114. Requires documentation for the method plus updates to the browser compatibility data.

Comment 14

[Connor Pearson]

Attachment: Bug 1764850 - Remove unused std::ops::Rem implementations. r=emilio

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/55fc19ba0ec4
Remove unused std::ops::Rem implementations. r=emilio

Comment 16

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/55fc19ba0ec4

Comment 17

[BugBot [:suhaib / :marco]]

A patch has been attached on this bug, which was already closed. Filing a separate bug will ensure better tracking. If this was not by mistake and further action is needed, please alert the appropriate party. (Or: if the patch doesn't change behavior -- e.g. landing a test case, or fixing a typo -- then feel free to disregard this message)