Skip to content

SVG image element not repainted when href attribute is removed#60612

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
Ahmad-S792:eng/SVG-image-element-not-repainted-when-href-attribute-is-removed
Mar 17, 2026
Merged

SVG image element not repainted when href attribute is removed#60612
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
Ahmad-S792:eng/SVG-image-element-not-repainted-when-href-attribute-is-removed

Conversation

@Ahmad-S792
Copy link
Contributor

@Ahmad-S792 Ahmad-S792 commented Mar 14, 2026

4046b9a

SVG image element not repainted when href attribute is removed
https://bugs.webkit.org/show_bug.cgi?id=309937
rdar://172530834

Reviewed by Nikolas Zimmermann.

When removing the href or xlink:href attribute from an SVG <image> element,
the image loader clears its cached image but the renderer is never
invalidated, so stale pixels remain on screen.

Add updateSVGRendererForElementChange() to the URI attribute change path
in SVGImageElement::svgAttributeChanged, matching the pattern used for
other visual attribute changes like preserveAspectRatio.

This is covered by image-remove-href-1.svg and image-remove-href-3.svg
test cases, which we fail as per WPT dashboard but they don't have any
failing expectations but with this patch, my local build show them as
passing and show 'green' rect rather than 'red'.

* Source/WebCore/svg/SVGImageElement.cpp:
(WebCore::SVGImageElement::svgAttributeChanged):

Canonical link: https://commits.webkit.org/309397@main

e66a5d9

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows Apple Internal
✅ 🧪 style ✅ 🛠 ios 🛠 mac ✅ 🛠 wpe 🛠 win ✅ 🛠 ios-apple
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug 🧪 wpe-wk2 🧪 win-tests loading 🛠 mac-apple
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 🧪 api-mac ✅ 🧪 api-wpe ✅ 🛠 vision-apple
🧪 ios-wk2-wpt 🧪 api-mac-debug ✅ 🛠 gtk3-libwebrtc
🧪 api-ios 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🛠 ios-safer-cpp 🧪 mac-wk2 🧪 gtk-wk2
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 🧪 merge 🛠 vision-sim 🧪 mac-wk2-stress 🛠 playstation
⏳ 🧪 vision-wk2 🧪 mac-intel-wk2
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
🛠 tv-sim
🛠 watch
🛠 watch-sim

@Ahmad-S792 Ahmad-S792 self-assigned this Mar 14, 2026
@Ahmad-S792 Ahmad-S792 added the SVG For bugs in the SVG implementation. label Mar 14, 2026
@Ahmad-S792
Copy link
Contributor Author

image

and

image

@Ahmad-S792
Copy link
Contributor Author

Ahmad-S792 commented Mar 14, 2026

I looked into whether this fixes failure on LBSE as well but I think it is more involved in case of LBSE. On LBSE, we don't render anything on these tests with and without patch.

@annevk
Copy link
Contributor

annevk commented Mar 14, 2026

Are we skipping these tests locally? Seems worth figuring out why it doesn’t reproduce on WKTR as otherwise it could regress again.

@Ahmad-S792
Copy link
Contributor Author

Are we skipping these tests locally? Seems worth figuring out why it doesn’t reproduce on WKTR as otherwise it could regress again.

I added this in commit message already:

This is covered by image-remove-href-1.svg and image-remove-href-3.svg
test cases, which we fail as per WPT dashboard but they don't have any
failing expectations but with this patch, my local build show them as
passing and show 'green' rect rather than 'red'.

I don't know why WKTR is doing this or not - we can investigate this separately, happy to file follow-up bug.

@Ahmad-S792
Copy link
Contributor Author

Filed for investigation - https://bugs.webkit.org/show_bug.cgi?id=309945

@annevk
Copy link
Contributor

annevk commented Mar 14, 2026

I saw that, but now is the best time to investigate. If this code gets refactored it might get quite involved.

@Ahmad-S792
Copy link
Contributor Author

I saw that, but now is the best time to investigate. If this code gets refactored it might get quite involved.

Can you elaborate on how this might get more involved. Because I think it is separate issue and has nothing to do with fixing the issue, where we fail tests.

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 14, 2026
@Ahmad-S792 Ahmad-S792 removed the merging-blocked Applied to prevent a change from being merged label Mar 14, 2026
Copy link
Contributor

@nikolaszimmermann nikolaszimmermann left a comment

Choose a reason for hiding this comment

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

LGTM.

@Ahmad-S792 Ahmad-S792 force-pushed the eng/SVG-image-element-not-repainted-when-href-attribute-is-removed branch from 28f6d78 to e66a5d9 Compare March 17, 2026 10:56
@Ahmad-S792 Ahmad-S792 added the merge-queue Applied to send a pull request to merge-queue label Mar 17, 2026
https://bugs.webkit.org/show_bug.cgi?id=309937
rdar://172530834

Reviewed by Nikolas Zimmermann.

When removing the href or xlink:href attribute from an SVG <image> element,
the image loader clears its cached image but the renderer is never
invalidated, so stale pixels remain on screen.

Add updateSVGRendererForElementChange() to the URI attribute change path
in SVGImageElement::svgAttributeChanged, matching the pattern used for
other visual attribute changes like preserveAspectRatio.

This is covered by image-remove-href-1.svg and image-remove-href-3.svg
test cases, which we fail as per WPT dashboard but they don't have any
failing expectations but with this patch, my local build show them as
passing and show 'green' rect rather than 'red'.

* Source/WebCore/svg/SVGImageElement.cpp:
(WebCore::SVGImageElement::svgAttributeChanged):

Canonical link: https://commits.webkit.org/309397@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/SVG-image-element-not-repainted-when-href-attribute-is-removed branch from e66a5d9 to 4046b9a Compare March 17, 2026 11:20
@webkit-commit-queue
Copy link
Collaborator

Committed 309397@main (4046b9a): https://commits.webkit.org/309397@main

Reviewed commits have been landed. Closing PR #60612 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 4046b9a into WebKit:main Mar 17, 2026
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Mar 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

SVG For bugs in the SVG implementation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants