Details

Reviewers

Commits

rMOZILLACENTRAL9d8a752ffbef: Bug 1809005 - Make scroll-timeline-{name|axis} be a coordinating list property…

Bugzilla Bug ID

Summary

Named scroll progress timelines are declared in the coordinated value list
constructed from the longhands of the scroll-timeline shorthand property,
which form a coordinating list property group with scroll-timeline-name as
the coordinating list base property.

In the meantime, we also update its shorthand to match the current spec.

Diff Detail

Repository

rMOZILLACENTRAL mozilla-central

Build Status

Buildable 504593
Build 600935: arc lint + arc unit

Event Timeline

boris created this revision.Jan 11 2023, 10:34 PM

Herald added a project: secure-revision. · View Herald TranscriptJan 11 2023, 10:34 PM

Harbormaster completed remote builds in B495747: Diff 667315.Jan 11 2023, 10:34 PM

phab-bot published this revision for review.Jan 11 2023, 10:34 PM

phab-bot changed the visibility from "Custom Policy" to "Public (No Login Required)".

phab-bot changed the edit policy from "Custom Policy" to "Restricted Project (Project)".

phab-bot removed a project: secure-revision.

Herald added a subscriber: emilio. · View Herald TranscriptJan 11 2023, 10:34 PM

boris updated this revision to Diff 667317.Jan 11 2023, 10:37 PM

Harbormaster completed remote builds in B495749: Diff 667317.Jan 11 2023, 10:37 PM

boris updated this revision to Diff 667372.Jan 11 2023, 11:42 PM

Harbormaster completed remote builds in B495784: Diff 667372.Jan 11 2023, 11:42 PM

boris updated this revision to Diff 667409.Jan 12 2023, 1:13 AM

Harbormaster completed remote builds in B495818: Diff 667409.Jan 12 2023, 1:13 AM

boris updated this revision to Diff 670862.Jan 19 2023, 11:44 PM

Harbormaster completed remote builds in B498693: Diff 670862.Jan 19 2023, 11:44 PM

boris added reviewers: Restricted Project, layout-reviewers.Jan 19 2023, 11:44 PM

emilio requested changes to this revision.Jan 20 2023, 8:13 PM

emilio added inline comments.

dom/animation/ScrollTimeline.cpp
118	This seems a drive-by change, maybe worth a separate patch?
122	If we change the code above to use the flat tree, we also need to change this.
130	Let's use `for (const auto& timeline : ...) {`?
layout/style/nsStyleStruct.h
1292	Same, let's do `mAxis = StyleScrollAxis::Block;` so that we don't have uninit values.
servo/components/style/properties/shorthands/ui.mako.rs
337	Probably worth a `Vec::with_capacity(1)` perhaps?
343	`unwrap_or_default()`?
368	I think you can just remove this condition, for two reasons: I think it cannot happen. Even if it did the behavior would be correct.

This revision now requires changes to proceed.Jan 20 2023, 8:13 PM

Rebased and addressed comments

boris removed a parent revision: D166595: Bug 1804573 - Update scroll-timeline shorthand syntax in WPT..Jan 25 2023, 7:57 PM

Harbormaster completed remote builds in B500511: Diff 673235.Jan 25 2023, 7:57 PM

emilio accepted this revision as: emilio.Jan 26 2023, 11:27 AM

emilio added a project: testing-approved.

emilio removed reviewers: Restricted Project, layout-reviewers.

emilio added inline comments.

servo/components/style/properties/shorthands/ui.mako.rs
379	write_char(' ')?

This revision is now accepted and ready to land.Jan 26 2023, 11:27 AM

Addresed comments

Harbormaster completed remote builds in B500990: Diff 673877.Jan 26 2023, 8:04 PM

Closed by commit rMOZILLACENTRAL9d8a752ffbef: Bug 1809005 - Make scroll-timeline-{name|axis} be a coordinating list property… (authored by boris). · Explain WhyJan 26 2023, 11:26 PM

This revision was automatically updated to reflect the committed changes.

boris added a commit: rMOZILLACENTRAL9d8a752ffbef: Bug 1809005 - Make scroll-timeline-{name|axis} be a coordinating list property….

Bug 1809005 - Make scroll-timeline-{name|axis} be a coordinating list property group.
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 674002

dom/animation/ScrollTimeline.cpp

layout/style/GeckoBindings.h

layout/style/GeckoBindings.cpp

layout/style/nsStyleStruct.h

layout/style/nsStyleStruct.cpp

layout/style/test/property_database.js

servo/components/style/gecko_bindings/sugar/ns_style_auto_array.rs

servo/components/style/properties/gecko.mako.rs

servo/components/style/properties/longhands/ui.mako.rs

servo/components/style/properties/shorthands/ui.mako.rs

testing/web-platform/meta/scroll-animations/css/scroll-timeline-shorthand.tentative.html.ini

testing/web-platform/tests/scroll-animations/css/scroll-timeline-axis-computed.html

testing/web-platform/tests/scroll-animations/css/scroll-timeline-axis-computed.tentative.html

testing/web-platform/tests/scroll-animations/css/scroll-timeline-axis-parsing.html

testing/web-platform/tests/scroll-animations/css/scroll-timeline-axis-parsing.tentative.html

testing/web-platform/tests/scroll-animations/css/scroll-timeline-name-computed.html

testing/web-platform/tests/scroll-animations/css/scroll-timeline-name-computed.tentative.html

testing/web-platform/tests/scroll-animations/css/scroll-timeline-name-parsing.html

testing/web-platform/tests/scroll-animations/css/scroll-timeline-name-parsing.tentative.html

Bug 1809005 - Make scroll-timeline-{name|axis} be a coordinating list property group.ClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 674002

dom/animation/ScrollTimeline.cpp

layout/style/GeckoBindings.h

layout/style/GeckoBindings.cpp

layout/style/nsStyleStruct.h

layout/style/nsStyleStruct.cpp

layout/style/test/property_database.js

servo/components/style/gecko_bindings/sugar/ns_style_auto_array.rs

servo/components/style/properties/gecko.mako.rs

servo/components/style/properties/longhands/ui.mako.rs

servo/components/style/properties/shorthands/ui.mako.rs

testing/web-platform/meta/scroll-animations/css/scroll-timeline-shorthand.tentative.html.ini

testing/web-platform/tests/scroll-animations/css/scroll-timeline-axis-computed.html

testing/web-platform/tests/scroll-animations/css/scroll-timeline-axis-computed.tentative.html

testing/web-platform/tests/scroll-animations/css/scroll-timeline-axis-parsing.html

testing/web-platform/tests/scroll-animations/css/scroll-timeline-axis-parsing.tentative.html

testing/web-platform/tests/scroll-animations/css/scroll-timeline-name-computed.html

testing/web-platform/tests/scroll-animations/css/scroll-timeline-name-computed.tentative.html

testing/web-platform/tests/scroll-animations/css/scroll-timeline-name-parsing.html

testing/web-platform/tests/scroll-animations/css/scroll-timeline-name-parsing.tentative.html

Bug 1809005 - Make scroll-timeline-{name|axis} be a coordinating list property group.
ClosedPublic
Actions

Revision Contents
Changeset List