Skip to content

script: Iterate over correct collections when moving#44347

Merged
jdm merged 1 commit into
servo:mainfrom
TimvdLippe:underline-correct-html
Apr 19, 2026
Merged

script: Iterate over correct collections when moving#44347
jdm merged 1 commit into
servo:mainfrom
TimvdLippe:underline-correct-html

Conversation

@TimvdLippe

Copy link
Copy Markdown
Contributor

Several times, we first need to capture the relevant nodes, perform a step and then iterate over the nodes. Previously, we would iterate over it, but the iterator could have moved, hence we would jump over some nodes.

Instead, capture these beforehand and then iterate over them.

Also fixes the issue where text-decoration wasn't properly checked, since its a shorthand for 3 longhands. And a PropertyDeclarationBlock only has longhands.

The two regressions are for tests that now generate the correct HTML, but their ranges are incorrectly moved from the a to the b. I went through the relevant algorithms and didn't spot the mistake. To keep on making babysteps towards a full implementation, I will tackle these in a follow-up. These algorithms are already difficult enough to reason about.

Part of #25005

Testing: WPT

Several times, we first need to capture the relevant
nodes, perform a step and then iterate over the nodes.
Previously, we would iterate over it, but the iterator
could have moved, hence we would jump over some nodes.

Instead, capture these beforehand and then iterate over them.

Also fixes the issue where text-decoration wasn't properly
checked, since its a shorthand for 3 longhands. And a
PropertyDeclarationBlock only has longhands.

The two regressions are for tests that now generate the
correct HTML, but their ranges are incorrectly moved from
the `a` to the `b`. I went through the relevant algorithms
and didn't spot the mistake. To keep on making babysteps
towards a full implementation, I will tackle these in a
follow-up. These algorithms are already difficult enough
to reason about.

Part of servo#25005

Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
@TimvdLippe TimvdLippe requested a review from gterzian as a code owner April 19, 2026 08:59
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Apr 19, 2026
@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Apr 19, 2026
@jdm jdm added this pull request to the merge queue Apr 19, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 19, 2026
Merged via the queue into servo:main with commit 4184b11 Apr 19, 2026
33 checks passed
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants