Closed Bug 2024014 Opened 8 months ago Closed 7 months ago

Implement multi-value support in StylePropertyMapReadOnly::GetAll

Categories

(Core :: CSS Parsing and Computation, task)

Product:
Core
Component:
CSS Parsing and Computation
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
151 Branch
Tracking Flags:
Tracking Status
firefox151 --- fixed

People

(Reporter: janv, Assigned: janv)

References

(Blocks 1 open bug)

Details

Attachments

(7 external files)

StylePropertyMapReadOnly::GetAll currently has a simplified implementation that does not fully support returning multiple values.

This bug aims to improve GetAll to properly handle multi-value reification and return all applicable values for a given property.

Blocks: 2024023

Change Servo_DeclarationBlock_GetPropertyTypedValue and
Servo_ComputedValues_GetPropertyTypedValue to take a CSSPropertyId instead of
nsACString.

This forces callers to resolve and validate the property up front, so these
helpers only fail in unexpected cases. That makes result handling clearer for
follow-up GetAll work, where it is important to distinguish between properties
with no value and unexpected errors.

Some higher-level helpers are updated accordingly. This should also be
slightly more efficient by avoiding repeated property lookup.

Servo_ComputedValues_GetPropertyTypedValue currently treats custom properties
without implementation support as an unexpected error.

Update the behavior to treat such cases as if the property has no value
instead. This aligns with the intended semantics for GetAll, where it is
important to distinguish between missing values and unexpected failures.

Now that missing values and unexpected errors are handled separately,
unexpected errors from typed value retrieval helpers can be propagated.

This makes error handling simpler and more consistent.

StylePropertyMapReadOnly::GetAll previously forwarded to Get, which was
sufficient for some time.

Now that the underlying infrastructure is in place, implement GetAll to
explicitly follow the spec and prepare for returning multiple typed values
from helpers.

Switch typed value retrieval helpers from returning a single TypedValue to
returning a newly introduced TypedValueList.

This leverages the existing Stylo infrastructure for reifying multiple values
and makes StylePropertyMapReadOnly::GetAll return multiple values when
possible.

The PropertyTypedValue enum is intentionally not renamed in this patch. That
cleanup is done separately to make the core behavior change easier to review.

Rename PropertyTypedValue to PropertyTypedValueList to reflect that typed
value retrieval now returns multiple values.

This is a follow-up to the previous patch that switched the underlying
representation from TypedValue to TypedValueList.

Keywords: leave-open
Pushed by jvarga@igalia.com: https://github.com/mozilla-firefox/firefox/commit/a0dbf0578a9c https://hg.mozilla.org/integration/autoland/rev/5e0d16ac8df9 Use CSSPropertyId in typed value retrieval helpers; r=emilio,firefox-style-system-reviewers https://github.com/mozilla-firefox/firefox/commit/3bd8c687752e https://hg.mozilla.org/integration/autoland/rev/0d9ff7aed208 Treat unimplemented custom properties as missing in computed typed value retrieval; r=emilio,firefox-style-system-reviewers
Attachment #9554023 - Attachment description: Bug 2024014 - Propagate unexpected errors from typed value retrieval helpers; r=emilio → Bug 2024014 - Propagate errors from typed value retrieval helpers; r=emilio

Remove unnecessary uses of unsafe where they can be avoided, and limit the
scope of unsafe blocks where they are still required.

Pushed by jvarga@igalia.com: https://github.com/mozilla-firefox/firefox/commit/52905294ee8c https://hg.mozilla.org/integration/autoland/rev/bac0a26875ac Propagate errors from typed value retrieval helpers; r=emilio,firefox-style-system-reviewers
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/58902 for changes under testing/web-platform/tests
Upstream PR merged by moz-wptsync-bot
Pushed by jvarga@igalia.com: https://github.com/mozilla-firefox/firefox/commit/715556b1dcce https://hg.mozilla.org/integration/autoland/rev/9856b9fe233a Implement StylePropertyMapReadOnly::GetAll following spec steps; r=emilio,firefox-style-system-reviewers https://github.com/mozilla-firefox/firefox/commit/025465a0d79c https://hg.mozilla.org/integration/autoland/rev/bee4f75ab557 Return TypedValueList from typed value retrieval helpers; r=emilio,firefox-style-system-reviewers
Keywords: leave-open
Pushed by jvarga@igalia.com: https://github.com/mozilla-firefox/firefox/commit/9d697bd13a97 https://hg.mozilla.org/integration/autoland/rev/73f04f0aa7a0 Rename PropertyTypedValue to PropertyTypedValueList; r=emilio,firefox-style-system-reviewers https://github.com/mozilla-firefox/firefox/commit/551eb326dff7 https://hg.mozilla.org/integration/autoland/rev/26b19d11f6e2 Remove unnecessary unsafe usage and tighten scope where possible; r=emilio,firefox-style-system-reviewers

https://hg.mozilla.org/mozilla-central/rev/73f04f0aa7a0
https://hg.mozilla.org/mozilla-central/rev/26b19d11f6e2

Status: ASSIGNED → RESOLVED
Closed: 7 months ago
status-firefox151: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 151 Branch
QA Whiteboard: [qa-triage-done-c152/b151]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: