Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
SVG fragment reference fails in shadow tree under some circumstances
https://bugs.webkit.org/show_bug.cgi?id=212820

Reviewed by Darin Adler.

The bug was caused by SVG fragment references looking up elements in the document tree instead of in each shadow tree.
This patch moves the lookup of SVG fragment references to each tree scope. However, when such a reference appears in
the content referenced by a SVG use element, we must do the lookup in the tree in which SVG use element resides and
not SVG use element's shadow tree. For this reason, this patch introduces Node::treeScopeForSVGReferences which
abstracts away this concept as well as RenderObject::treeScopeForSVGReferences which calls the former.

* LayoutTests/fast/shadow-dom/svg-multiple-references-in-multiple-shadows-linear-gradient-expected.html: Added.
* LayoutTests/fast/shadow-dom/svg-multiple-references-in-multiple-shadows-linear-gradient.html: Added.
* LayoutTests/fast/shadow-dom/svg-multiple-references-in-multiple-shadows-radial-gradient-expected.html: Added.
* LayoutTests/fast/shadow-dom/svg-multiple-references-in-multiple-shadows-radial-gradient.html: Added.
* LayoutTests/fast/shadow-dom/svg-multiple-references-in-multiple-shadows-rect-expected.html: Added.
* LayoutTests/fast/shadow-dom/svg-multiple-references-in-multiple-shadows-rect.html: Added.
* Source/WebCore/accessibility/AccessibilitySVGElement.cpp:
(WebCore::AccessibilitySVGElement::targetForUseElement const):
* Source/WebCore/dom/Node.cpp:
(WebCore::Node::treeScopeForSVGReferences const):
* Source/WebCore/dom/Node.h:
* Source/WebCore/dom/TreeScope.cpp:
(WebCore::TreeScope::svgResourcesMap const):
(WebCore::TreeScope::addSVGResource):
(WebCore::TreeScope::removeSVGResource):
(WebCore::TreeScope::svgResourceById const):
(WebCore::TreeScope::addPendingSVGResource):
(WebCore::TreeScope::isIdOfPendingSVGResource const):
(WebCore::TreeScope::isElementWithPendingSVGResources const):
(WebCore::TreeScope::isPendingSVGResource const):
(WebCore::TreeScope::clearHasPendingSVGResourcesIfPossible):
(WebCore::TreeScope::removeElementFromPendingSVGResources):
(WebCore::TreeScope::removePendingSVGResource):
(WebCore::TreeScope::markPendingSVGResourcesForRemoval):
(WebCore::TreeScope::takeElementFromPendingSVGResourcesForRemovalMap):
* Source/WebCore/dom/TreeScope.h:
* Source/WebCore/rendering/CSSFilter.cpp:
(WebCore::referenceFilterElement):
* Source/WebCore/rendering/ReferencedSVGResources.cpp:
(WebCore::ReferencedSVGResources::~ReferencedSVGResources):
(WebCore::ReferencedSVGResources::removeClientForTarget):
(WebCore::ReferencedSVGResources::updateReferencedResources):
(WebCore::ReferencedSVGResources::elementForResourceID):
(WebCore::ReferencedSVGResources::referencedFilterElement):
(WebCore::ReferencedSVGResources::referencedClipperRenderer):
* Source/WebCore/rendering/ReferencedSVGResources.h:
* Source/WebCore/rendering/RenderElement.cpp:
(WebCore::RenderElement::updateReferencedSVGResources):
* Source/WebCore/rendering/RenderLayer.cpp:
(WebCore::RenderLayer::setupClipPath):
* Source/WebCore/rendering/RenderObject.h:
(WebCore::RenderObject::treeScopeForSVGReferences const):
* Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp:
(WebCore::RenderSVGResourceContainer::willBeDestroyed):
(WebCore::RenderSVGResourceContainer::idChanged):
(WebCore::RenderSVGResourceContainer::registerResource):
* Source/WebCore/rendering/svg/RenderSVGResourceContainer.h:
(WebCore::getRenderSVGResourceContainerById):
(WebCore::getRenderSVGResourceById):
* Source/WebCore/rendering/svg/RenderSVGTextPath.cpp:
(WebCore::RenderSVGTextPath::targetElement const):
* Source/WebCore/rendering/svg/SVGRenderTreeAsText.cpp:
(WebCore::writeResources):
* Source/WebCore/rendering/svg/SVGResources.cpp:
(WebCore::paintingResourceFromSVGPaint):
(WebCore::SVGResources::buildCachedResources):
(WebCore::registerPendingResource): Deleted.
* Source/WebCore/rendering/svg/SVGResourcesCache.cpp:
(WebCore::SVGResourcesCache::resourceDestroyed):
* Source/WebCore/style/StyleBuilderConverter.h:
(WebCore::Style::BuilderConverter::convertPathOperation):
* Source/WebCore/svg/SVGDocumentExtensions.cpp:
(WebCore::SVGDocumentExtensions::addResource): Deleted.
(WebCore::SVGDocumentExtensions::removeResource): Deleted.
(WebCore::SVGDocumentExtensions::resourceById const): Deleted.
(WebCore::SVGDocumentExtensions::addPendingResource): Deleted.
(WebCore::SVGDocumentExtensions::isIdOfPendingResource const): Deleted.
(WebCore::SVGDocumentExtensions::isElementWithPendingResources const): Deleted.
(WebCore::SVGDocumentExtensions::isPendingResource const): Deleted.
(WebCore::SVGDocumentExtensions::clearHasPendingResourcesIfPossible): Deleted.
(WebCore::SVGDocumentExtensions::removeElementFromPendingResources): Deleted.
(WebCore::SVGDocumentExtensions::markPendingResourcesForRemoval): Deleted.
(WebCore::SVGDocumentExtensions::takeElementFromPendingResourcesForRemovalMap): Deleted.
* Source/WebCore/svg/SVGDocumentExtensions.h:
(WebCore::SVGDocumentExtensions::removePendingResource): Deleted.
* Source/WebCore/svg/SVGElement.cpp:
(WebCore::SVGElement::~SVGElement):
(WebCore::SVGElement::removedFromAncestor):
(WebCore::SVGElement::insertedIntoAncestor):
(WebCore::SVGElement::buildPendingResourcesIfNeeded):
* Source/WebCore/svg/SVGFEImageElement.cpp:
(WebCore::SVGFEImageElement::buildPendingResource):
(WebCore::SVGFEImageElement::imageBufferForEffect const):
* Source/WebCore/svg/SVGLinearGradientElement.cpp:
(WebCore::SVGLinearGradientElement::collectGradientAttributes):
* Source/WebCore/svg/SVGMPathElement.cpp:
(WebCore::SVGMPathElement::buildPendingResource):
(WebCore::SVGMPathElement::pathElement):
* Source/WebCore/svg/SVGRadialGradientElement.cpp:
(WebCore::SVGRadialGradientElement::collectGradientAttributes):
* Source/WebCore/svg/SVGTRefElement.cpp:
(WebCore::SVGTRefElement::detachTarget):
(WebCore::SVGTRefElement::buildPendingResource):
* Source/WebCore/svg/SVGTextPathElement.cpp:
(WebCore::SVGTextPathElement::buildPendingResource):
* Source/WebCore/svg/SVGUseElement.cpp:
(WebCore::SVGUseElement::updateUserAgentShadowTree):
* Source/WebCore/svg/animation/SVGSMILElement.cpp:
(WebCore::SVGSMILElement::buildPendingResource):

Canonical link: https://commits.webkit.org/265565@main
  • Loading branch information
rniwa committed Jun 27, 2023
1 parent cb31f48 commit 9057e67
Show file tree
Hide file tree
Showing 35 changed files with 481 additions and 267 deletions.
@@ -0,0 +1,21 @@
<!DOCTYPE html>
<html lang="en">
<head>
<style>
div {
width: 100px;
height: 100px;
}
</style>
</head>
<body>
<div>
<svg>
<linearGradient id="gradient">
<stop offset="0%" stop-color="green" />
</linearGradient>
<rect id="rect" width="100" height="100" fill="url(#gradient)">
</svg>
</div>
</body>
</html>
@@ -0,0 +1,42 @@
<!DOCTYPE html>
<html lang="en">
<head>
<style>
div {
width: 100px;
height: 100px;
}
</style>
<script>
if (window.testRunner)
testRunner.waitUntilDone();
window.onload = () => {
const shadowRoot1 = container1.attachShadow({mode: "closed"});
shadowRoot1.appendChild(template.content.cloneNode(true));

const shadowRoot2 = container2.attachShadow({mode: "closed"});
shadowRoot2.appendChild(template.content.cloneNode(true));

requestAnimationFrame(() => {
container2.style.display = "none";
shadowRoot1.querySelector("rect").style.filter = "url(#flood-color)";
if (window.testRunner)
testRunner.notifyDone();
});
}
</script>
</head>
<body>
<template id="template">
<svg>
<linearGradient id="gradient">
<stop offset="0%" stop-color="green" />
</linearGradient>
<linearGradient id="gradientUse" fill="red" href="#gradient"></linearGradient>
<rect id="rect" width="100" height="100" fill="url(#gradientUse)">
</svg>
</template>
<div id="container1"></div>
<div id="container2"></div>
</body>
</html>
@@ -0,0 +1,21 @@
<!DOCTYPE html>
<html lang="en">
<head>
<style>
div {
width: 100px;
height: 100px;
}
</style>
</head>
<body>
<div>
<svg>
<radialGradient id="gradient">
<stop offset="0%" stop-color="green" />
</radialGradient>
<rect id="rect" width="100" height="100" fill="url(#gradient)">
</svg>
</div>
</body>
</html>
@@ -0,0 +1,42 @@
<!DOCTYPE html>
<html lang="en">
<head>
<style>
div {
width: 100px;
height: 100px;
}
</style>
<script>
if (window.testRunner)
testRunner.waitUntilDone();
window.onload = () => {
const shadowRoot1 = container1.attachShadow({mode: "closed"});
shadowRoot1.appendChild(template.content.cloneNode(true));

const shadowRoot2 = container2.attachShadow({mode: "closed"});
shadowRoot2.appendChild(template.content.cloneNode(true));

requestAnimationFrame(() => {
container2.style.display = "none";
shadowRoot1.querySelector("rect").style.filter = "url(#flood-color)";
if (window.testRunner)
testRunner.notifyDone();
});
}
</script>
</head>
<body>
<template id="template">
<svg>
<radialGradient id="gradient">
<stop offset="0%" stop-color="green" />
</radialGradient>
<radialGradient id="gradientUse" fill="red" href="#gradient"></radialGradient>
<rect id="rect" width="100" height="100" fill="url(#gradientUse)">
</svg>
</template>
<div id="container1"></div>
<div id="container2"></div>
</body>
</html>
@@ -0,0 +1,15 @@
<!DOCTYPE html>
<html lang="en">
<head>
<style>
div {
width: 100px;
height: 100px;
background: green;
}
</style>
</head>
<body>
<div></div>
</body>
</html>
@@ -0,0 +1,41 @@
<!DOCTYPE html>
<html lang="en">
<head>
<style>
div {
width: 100px;
height: 100px;
}
</style>
<script>
if (window.testRunner)
testRunner.waitUntilDone();
window.onload = () => {
const shadowRoot1 = container1.attachShadow({mode: "closed"});
shadowRoot1.appendChild(template.content.cloneNode(true));

const shadowRoot2 = container2.attachShadow({mode: "closed"});
shadowRoot2.appendChild(template.content.cloneNode(true));

requestAnimationFrame(() => {
container2.style.display = "none";
shadowRoot1.querySelector("rect").style.filter = "url(#flood-color)";
if (window.testRunner)
testRunner.notifyDone();
});
}
</script>
</head>
<body>
<template id="template">
<svg>
<filter id="flood-color">
<feFlood width="110" height="110" flood-color="green"/>
</filter>
<rect width="100" height="100" fill="red"/>
</svg>
</template>
<div id="container1"></div>
<div id="container2"></div>
</body>
</html>
2 changes: 1 addition & 1 deletion Source/WebCore/accessibility/AccessibilitySVGElement.cpp
Expand Up @@ -69,7 +69,7 @@ AccessibilityObject* AccessibilitySVGElement::targetForUseElement() const
if (href.isEmpty())
href = getAttribute(HTMLNames::hrefAttr);

auto target = SVGURIReference::targetElementFromIRIString(href, use.treeScope());
auto target = SVGURIReference::targetElementFromIRIString(href, use.treeScopeForSVGReferences());
if (!target.element)
return nullptr;

Expand Down
12 changes: 12 additions & 0 deletions Source/WebCore/dom/Node.cpp
Expand Up @@ -60,6 +60,7 @@
#include "LocalFrameView.h"
#include "Logging.h"
#include "MutationEvent.h"
#include "NodeName.h"
#include "NodeRareDataInlines.h"
#include "NodeRenderStyle.h"
#include "PointerEvent.h"
Expand Down Expand Up @@ -1308,6 +1309,17 @@ Element* Node::parentElementInComposedTree() const
return nullptr;
}

TreeScope& Node::treeScopeForSVGReferences() const
{
if (auto* shadowRoot = containingShadowRoot(); shadowRoot && shadowRoot->mode() == ShadowRootMode::UserAgent) {
if (shadowRoot->host() && shadowRoot->host()->elementName() == ElementNames::SVG::use) {
ASSERT(m_treeScope->parentTreeScope());
return *m_treeScope->parentTreeScope();
}
}
return treeScope();
}

bool Node::isInUserAgentShadowTree() const
{
auto* shadowRoot = containingShadowRoot();
Expand Down
2 changes: 2 additions & 0 deletions Source/WebCore/dom/Node.h
Expand Up @@ -390,6 +390,8 @@ class Node : public EventTarget {
void setTreeScopeRecursively(TreeScope&);
static ptrdiff_t treeScopeMemoryOffset() { return OBJECT_OFFSETOF(Node, m_treeScope); }

TreeScope& treeScopeForSVGReferences() const;

// Returns true if this node is associated with a document and is in its associated document's
// node tree, false otherwise (https://dom.spec.whatwg.org/#connected).
bool isConnected() const { return hasNodeFlag(NodeFlag::IsConnected); }
Expand Down

0 comments on commit 9057e67

Please sign in to comment.