1905604 - initial-value from registered property is not picked when property is set on a parent element, is invalid-at-computed-value-time, and has font-relative dependencies

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Nicolas Chevobbe [:nchevobbe][PTO back September 7]]

Steps to reproduce

1. Navigate to data:text/html,<meta charset=utf8><style>@property --a { syntax: '<color>'; inherits: true; initial-value: blue; } body {--a: 1em;} h1 { color: var(--a); } </style><h1>hello</h1>

Here's the pretty-printed CSS:

@property --a {
  syntax: '<color>';
  inherits: true;
  initial-value: blue;
}
body {
  --a: 1em;
}
h1 {
  color: var(--a);
}

Expected results

The text is blue, the registered property initial value

Actual results

The text is black, but is blue in Chrome and Safari

---

We're defining a custom color registered property, and we set its value to 1em on the body. The declaration is invalid at computed value time here, as it does not match the declared property syntax. My understanding is that in such case, we should default to the registered property initial-value, which doesn't seem to be the case here.

Comment 1

[Tess Hoffman [:zrhoffman]]

When opening the testcase on a debug build, I get an assertion failure:

Hit MOZ_CRASH(Should be already computed) at servo/components/style/custom_properties.rs:2028

After bisecting, this seems to be regressed by the fix for bug 1899272.

Comment 2

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 1899272

Comment 3

[Emilio Cobos Álvarez [:emilio]]

Attachment: Test-case from comment 0

Comment 4

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1905604 - Fix edge case with registered color computation. r=#style

The issue is that we track the 1em as a custom reference and thus fail
to compute the color. But in this case 1em is not a valid value and we
should fall back to the initial value.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d33a0841dc39
Fix edge case with registered color computation. r=firefox-style-system-reviewers,devtools-reviewers,nchevobbe,zrhoffman

Comment 6

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

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

Comment 7

[Norisz Fay [:noriszfay]]

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

Comment 8

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

Upstream PR merged by moz-wptsync-bot

Comment 9

[BugBot [:suhaib / :marco]]

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox129 to wontfix.

For more information, please visit BugBot documentation.

Comment 10

[Emilio Cobos Álvarez [:emilio]]

Comment on attachment 9411754 [details]
Bug 1905604 - Fix edge case with registered color computation. r=#style

Beta/Release Uplift Approval Request

• User impact if declined: incorrect styling in some edge cases.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: comment 0
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Relatively local / isolated change to part of the code with pretty good test coverage.
• String changes made/needed: none
• Is Android affected?: Yes

Comment 11

[Donal Meehan [:dmeehan]]

Comment on attachment 9411754 [details]
Bug 1905604 - Fix edge case with registered color computation. r=#style

Approved for 129.0b3

Comment 12

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/858abecee3d8

Comment 13

[Donal Meehan [:dmeehan]]

:emilio, how about esr128, should it be uplifted there too?
How about release? What is the severity on this, if should it be considered in a ride-along for Fx128 or ride the train with Fx129?

Comment 14

[Emilio Cobos Álvarez [:emilio]]

I think it's probably a bit edge casey. If it comes up on the wild we should uplift but without that I'd tend not to.

Comment 15

[Ciprian Georgiu, Desktop QA]

I have reproduced this issue using an affected Nightly build (2024-07-01), on Win 11.

The issue is verified as fixed on latest Nightly 130.0a1 and Beta 129.0b3, running Win 11, Ubuntu 20.04 x64 and macOS 14.