Allow SVG dominant-baseline and text-orientation presentation attributes - #1526
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates DOMPurify’s SVG attribute allow-list to preserve two safe SVG2 text presentation attributes (dominant-baseline and text-orientation) that were previously stripped, and adds regression fixtures to prevent regressions.
Changes:
- Add
dominant-baselineandtext-orientationto the SVG allowed attributes list (src/attrs.ts). - Add two regression fixtures covering both attributes (
test/fixtures/expect.mjs). - Regenerate distribution bundles (
dist/*) to reflect the updated allow-list.
Reviewed changes
Copilot reviewed 2 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/fixtures/expect.mjs | Adds regression fixtures ensuring the two SVG attributes are preserved. |
| src/attrs.ts | Extends the SVG attribute allow-list with dominant-baseline and text-orientation. |
| dist/purify.js | Rebuilt distribution bundle incorporating the updated SVG allow-list. |
| dist/purify.es.mjs | Rebuilt ESM distribution bundle incorporating the updated SVG allow-list. |
| dist/purify.cjs.js | Rebuilt CJS distribution bundle incorporating the updated SVG allow-list. |
| dist/purify.min.js | Rebuilt minified distribution bundle incorporating the updated SVG allow-list. |
| dist/purify.min.js.map | Updated sourcemap corresponding to the rebuilt minified bundle. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
x00mario
approved these changes
Jul 10, 2026
Owner
|
Excellent, thank you 🙂 |
Owner
|
The changes look good, thanks @Develop-KIM . Would you mind adding signatures to your commit? Then we can merge. |
Both are pure presentation attributes from the W3C SVG2 text property adaptions but were missing from the SVG attribute allow-list, so DOMPurify stripped e.g. <text dominant-baseline="middle">. Their baseline siblings alignment-baseline and baseline-shift are already allow-listed; both added attributes are enum-only with no script/URL surface. Fixes #1525 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Develop-KIM
force-pushed
the
fix/svg-text-adaption-attrs
branch
from
July 10, 2026 16:01
7c54eb9 to
fcc22c6
Compare
Contributor
Author
|
Signed the commit, thanks |
Owner
|
Super, thank you! |
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1525.
dominant-baselinewas missing from the SVG attribute allow-list, so DOMPurify stripped it (e.g.<text dominant-baseline="middle">), even though its baseline siblingsalignment-baselineandbaseline-shiftare already allowed. This adds it, plustext-orientationfrom the same W3C SVG2 text property adaptions group. Both are enum-only presentation attributes with no script or URL surface.Before / after (default config):
You mentioned being happy to add a few safe ones in bulk. I checked the rest of that spec section and deliberately left out
unicode-bidi(itsbidi-overridevalue is a text-reordering/spoofing surface, so it felt like your call) and the deprecatedglyph-orientation-horizontal/glyph-orientation-vertical. Happy to add any of those if you'd like.Added two regression fixtures to
test/fixtures/expect.mjs. Lint and the jsdom suite pass locally.Implemented with AI assistance; I've reviewed and verified every change.