Skip to content

fix(removeUnknownsAndDefaults): don't strip inherited attrs across ID boundaries - #2241

Open
victorvianna wants to merge 1 commit into
svg:mainfrom
victorvianna:fix
Open

victorvianna wants to merge 1 commit into
svg:mainfrom
victorvianna:fix

Conversation

@victorvianna

Copy link
Copy Markdown

Two independent uselessOverrides bugs, both from comparing an attribute against the parent's full computed style.

  1. Inheritance was resolved through the whole ancestor chain, but an element with an id (or inside <defs>/<symbol>) may be instantiated by <use> somewhere else entirely, where the ancestors above it do not apply. Stop the walk at that boundary, keeping the boundary element itself since it is cloned along with the subtree.

  2. The parent's own presentation attributes were all treated as inheritable. presentationNonInheritableGroupAttrs only covers eight of them, so overflow, vector-effect, stop-color, stop-opacity, flood-color, clip, transform-origin and friends leaked through and were dropped from children that legitimately carried them — e.g. overflow on a nested <svg>, which controls its clipping. Filter on inheritableAttrs instead.

Both views of the cascade are collected in a single ancestor walk. Each level re-matches the whole stylesheet, so walking twice doubled the cost of the plugin on deep documents.

…ss ID boundaries

Two independent uselessOverrides bugs, both from comparing an attribute
against the parent's full computed style.

1. Inheritance was resolved through the whole ancestor chain, but an
   element with an `id` (or inside `<defs>`/`<symbol>`) may be
   instantiated by `<use>` somewhere else entirely, where the ancestors
   above it do not apply. Stop the walk at that boundary, keeping the
   boundary element itself since it is cloned along with the subtree.

2. The parent's own presentation attributes were all treated as
   inheritable. `presentationNonInheritableGroupAttrs` only covers eight
   of them, so `overflow`, `vector-effect`, `stop-color`, `stop-opacity`,
   `flood-color`, `clip`, `transform-origin` and friends leaked through
   and were dropped from children that legitimately carried them —
   e.g. `overflow` on a nested `<svg>`, which controls its clipping.
   Filter on `inheritableAttrs` instead.

Both views of the cascade are collected in a single ancestor walk.
Each level re-matches the whole stylesheet, so walking twice doubled the
cost of the plugin on deep documents.
@victorvianna

Copy link
Copy Markdown
Author

@TrySound Could you please approve the workflow runs when you have a chance? Happy to address any feedback.

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.

1 participant