Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

REGRESSION (r238524): SVG textPath cannot be rendered when <text> element is referred by <use> element #2491

Merged
merged 1 commit into from Jul 17, 2022
Merged

REGRESSION (r238524): SVG textPath cannot be rendered when <text> element is referred by <use> element #2491

merged 1 commit into from Jul 17, 2022

Conversation

rniwa
Copy link
Member

@rniwa rniwa commented Jul 16, 2022 •
edited by webkit-commit-queue

72c9014

REGRESSION (r238524): SVG textPath cannot be rendered when <text> element is referred by <use> element
https://bugs.webkit.org/show_bug.cgi?id=198280

Reviewed by Myles C. Maxfield.

The bug was caused by targetElementFromIRIString looking for a matching element in the use element's
UA shadow tree instead of the outer tree which defined the content of use element's shadow tree.

Fixed the bug by looking for the matching element in the tree that contains the use element instead.

* LayoutTests/svg/dom/use-element-reference-within-use-shadow-tree-expected.html: Added.
* LayoutTests/svg/dom/use-element-reference-within-use-shadow-tree.html: Added.
* Source/WebCore/svg/SVGURIReference.cpp:
(WebCore::SVGURIReference::targetElementFromIRIString): Fixed the bug.

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

@rniwa rniwa self-assigned this Jul 16, 2022
@rniwa rniwa added SVG For bugs in the SVG implementation. WebKit Nightly Build labels Jul 16, 2022
@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label Jul 16, 2022
litherum
litherum approved these changes Jul 17, 2022
View reviewed changes
Copy link
Contributor

@litherum litherum left a comment

Choose a reason for hiding this comment

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

Makes sense to me.

@rniwa rniwa removed the merging-blocked Applied to prevent a change from being merged label Jul 17, 2022
@rniwa
Copy link
Member Author

rniwa commented Jul 17, 2022

fast/text/international/system-language/han-text-style.html is most certainly not regressed by this patch.

@rniwa rniwa added the merge-queue Applied to send a pull request to merge-queue label Jul 17, 2022
@litherum
Copy link
Contributor

litherum commented Jul 17, 2022 •
edited

fast/text/international/system-language/han-text-style.html is most certainly not regressed by this patch.

Correct. This test has been failing for a while on EWS. Please feel free to disregard the failure; I'm looking at it elsewhere.

@rniwa
Copy link
Member Author

rniwa commented Jul 17, 2022

fast/text/international/system-language/han-text-style.html is most certainly not regressed by this patch.

Correct. This test has been failing for a while on EWS. Please feel free to disregard the failure; I'm looking at it elsewhere.

Yeah, I noticed. We should probably mark as failing to speed up EWS / merge queue.

@rniwa
Copy link
Member Author

rniwa commented Jul 17, 2022

fast/text/international/system-language/han-text-style.html is most certainly not regressed by this patch.

Correct. This test has been failing for a while on EWS. Please feel free to disregard the failure; I'm looking at it elsewhere.

Yeah, I noticed. We should probably mark as failing to speed up EWS / merge queue.

This is tracked in https://bugs.webkit.org/show_bug.cgi?id=242840.

@rniwa
REGRESSION (r238524): SVG textPath cannot be rendered when <text> ele…
72c9014 
…ment is referred by <use> element

https://bugs.webkit.org/show_bug.cgi?id=198280

Reviewed by Myles C. Maxfield.

The bug was caused by targetElementFromIRIString looking for a matching element in the use element's
UA shadow tree instead of the outer tree which defined the content of use element's shadow tree.

Fixed the bug by looking for the matching element in the tree that contains the use element instead.

* LayoutTests/svg/dom/use-element-reference-within-use-shadow-tree-expected.html: Added.
* LayoutTests/svg/dom/use-element-reference-within-use-shadow-tree.html: Added.
* Source/WebCore/svg/SVGURIReference.cpp:
(WebCore::SVGURIReference::targetElementFromIRIString): Fixed the bug.

Canonical link: https://commits.webkit.org/252547@main
@webkit-early-warning-system webkit-early-warning-system merged commit 72c9014 into WebKit:main Jul 17, 2022
@webkit-commit-queue
Copy link
Collaborator

Committed 252547@main (72c9014): https://commits.webkit.org/252547@main

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

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jul 17, 2022
@rniwa rniwa deleted the fix198280 branch July 17, 2022 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@litherum litherum litherum approved these changes

@shallawa shallawa Awaiting requested review from shallawa

@alanbaradlay alanbaradlay Awaiting requested review from alanbaradlay

Assignees

@rniwa rniwa

Labels
SVG For bugs in the SVG implementation.
Projects
None yet
Milestone
No milestone
4 participants
@rniwa @litherum @webkit-commit-queue @webkit-early-warning-system