Skip to content

fix: clip-path export for absolute same-document URL references#580

Open
cedricbla wants to merge 2 commits intobubkoo:masterfrom
cedricbla:fix/clip-path-in-svg
Open

fix: clip-path export for absolute same-document URL references#580
cedricbla wants to merge 2 commits intobubkoo:masterfrom
cedricbla:fix/clip-path-in-svg

Conversation

@cedricbla
Copy link
Copy Markdown

Description

Fix clip-path rendering in exported images when source markup uses absolute same-document URL references.

This change normalizes clip-path URL values during cloning so references like url('/context.html#clip1') are rewritten to local fragment form url(#clip1) in the cloned tree, which survives export.

Covered sources:

  • SVG presentation attribute: clip-path="url(...)"
  • Inline style: style="clip-path: url(...)"

Motivation and Context

Fixes: #579

Problem:

  • html-to-image deep-clones DOM nodes before export.
  • Absolute clip-path URLs that target the same document can fail after cloning.
  • Result is incorrect output (missing clipping / disappearing elements).

Approach:

  • Detect clip-path url(...) values during clone/decorate.
  • If URL resolves to current document and includes a hash fragment, convert to url(#id).
  • Leave external references unchanged.

Outcome:

  • Exported output preserves clipping behavior for same-document absolute clip-path references.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Enhancement (changes that improvement of current feature or performance)
  • Refactoring (changes that neither fixes a bug nor adds a feature)
  • Test Case (changes that add missing tests or correct existing tests)
  • Code style optimization (changes that do not affect the meaning of the code)
  • Docs (changes that only update documentation)
  • Chore (changes that don't modify src or test files)

Self Check before Merge

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@biiibooo
Copy link
Copy Markdown
Contributor

biiibooo Bot commented Apr 1, 2026

👋 @cedricbla

💖 Thanks for opening this pull request! 💖

Please follow the contributing guidelines. And we use semantic commit messages to streamline the release process.

Examples of commit messages with semantic prefixes:

  • fix: don't overwrite prevent_default if default wasn't prevented
  • feat: add graph.scale() method
  • docs: graph.getShortestPath is now available

Things that will help get your PR across the finish line:

  • Follow the TypeScript coding style.
  • Run npm run lint locally to catch formatting errors earlier.
  • Document any user-facing changes you've made.
  • Include tests when adding/changing behavior.
  • Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 1, 2026

Codecov Report

❌ Patch coverage is 58.82353% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.50%. Comparing base (d9b2fcf) to head (8269999).
⚠️ Report is 8 commits behind head on master.

Files with missing lines Patch % Lines
src/clone-node.ts 58.82% 7 Missing and 7 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master     #580   +/-   ##
=======================================
  Coverage   66.50%   66.50%           
=======================================
  Files          10       10           
  Lines         612      612           
  Branches      150      150           
=======================================
  Hits          407      407           
  Misses        144      144           
  Partials       61       61           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown

@accesslint accesslint Bot left a comment

Choose a reason for hiding this comment

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

Found 3 issues across 3 rules.

@@ -0,0 +1,47 @@
<!DOCTYPE html>
<html lang="en">
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Best Practice: Page has no mechanism to bypass repeated content. Add a <main> landmark or skip link.

Page must have a mechanism to bypass repeated blocks of content.

Details

Missing: no landmarks (<main>, <nav>, <header>, <footer>), no skip link, no headings

Keyboard users must be able to skip repetitive content like navigation. Provide a skip link at the top of the page that links to the main content (e.g., <a href="#main">Skip to main content</a>), or use a <main> landmark. Screen readers can jump directly to landmarks, so a properly marked-up <main> element satisfies this requirement.


Best Practice: Page does not contain a level-one heading.

Page should contain a level-one heading.

Details

Page title: "SVG clip-path cases"

A level-one heading (<h1> or role='heading' with aria-level='1') helps users understand the page topic and provides a landmark for screen reader navigation. Each page should have exactly one h1 that describes the main content, typically matching or similar to the page title.


Best Practice: Page has no main landmark.

Page should have exactly one main landmark.

Details

The main landmark contains the primary content of the page. Screen readers allow users to jump directly to main content. Use a single <main> element (or role='main') to wrap the central content, excluding headers, footers, and navigation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clip-path URLs with absolute same-document references break in exported images

1 participant