Skip to content

Fix some correctness tests#127

Merged
noahbald merged 7 commits intonoahbald:mainfrom
devongovett:fixes
Apr 22, 2025
Merged

Fix some correctness tests#127
noahbald merged 7 commits intonoahbald:mainfrom
devongovett:fixes

Conversation

@devongovett
Copy link
Contributor

This reduces the w3c failures to 13. See individual commits. Most of them were fixed by the "fix computed styles" commit, which fixes how styles are inherited and resets computed styles on elements where use_style returns false.

Also fixed how the transform presentation attributes are parsed, which is different from CSS https://drafts.csswg.org/css-transforms/#svg-transform. In particular SVG allows spaces and commas in more places.

I wasn't sure about the remove unused ns tests which seemed incorrect to me, removing namespaces that are actually used. I updated the snapshots accordingly but wasn't sure if this was intentional.

The majority of the remaining test failures are in the MergePaths job. When this job is disabled, only 4 items are broken. Some of these also look identical to me in Chrome, so idk if maybe the comparison script is somehow rendering differently.

&context.stylesheet,
context.element_styles,
);
context.flags.set(ContextFlags::use_style, true);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the flag would already be true here, because

// use_style is derived from the flag
let use_style = context.flags.contains(ContextFlags::use_style);
// The branch expects `true`
if use_style && self.use_style(element) {

@noahbald
Copy link
Owner

Thanks for the work Devon, the changes make sense to me. I'm about to merge #128 which fixes some w3c test issues related to parsing/serializing, would you mind rebasing/merging when that's on main?

The majority of the remaining test failures are in the MergePaths job. When this job is disabled, only 4 items are broken. Some of these also look identical to me in Chrome, so idk if maybe the comparison script is somehow rendering differently.

@napi-rs/canvas uses Skia, which afaik is what Chrome is using for SVGs. I can check if something funky is going on maybe :\

@noahbald
Copy link
Owner

noahbald commented Apr 21, 2025

Most of them were fixed by the "fix computed styles" commit

Is there a snapshot test you could add for this somewhere?

@noahbald noahbald merged commit 6c7683a into noahbald:main Apr 22, 2025
20 checks passed
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.

2 participants