Skip to content

Commit

Permalink
REGRESSION(r258464): SVG use element doesn't render if it references …
Browse files Browse the repository at this point in the history
…a subsequent element after a style resolution

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

Reviewed by Said Abou-Hallawa.

The bug was caused by SVGPathElement::insertedIntoAncestor and a few other insertedIntoAncestor functions
ignoring the value returned by SVGElement::insertedIntoAncestor requesting a post insertion callback,
and SVGMPathElement::didFinishInsertingNode not calling SVGMPathElement::didFinishInsertingNode,
which updates SVG elements referencing the element.

This PR fixes all insertedIntoAncestor functions in SVG to respect the value returned by super class'
insertedIntoAncestor but many of them don't have any consequences at the moment.

* LayoutTests/svg/dom/use-element-refers-subsequent-image-element-after-style-update-expected.svg: Added.
* LayoutTests/svg/dom/use-element-refers-subsequent-image-element-after-style-update.svg: Added.
* LayoutTests/svg/dom/use-element-refers-subsequent-path-element-after-style-update-expected.svg: Added.
* LayoutTests/svg/dom/use-element-refers-subsequent-path-element-after-style-update.svg: Added.
* Source/WebCore/svg/SVGFEImageElement.cpp:
(WebCore::SVGFEImageElement::didFinishInsertingNode):
* Source/WebCore/svg/SVGFontFaceElement.cpp:
(WebCore::SVGFontFaceElement::insertedIntoAncestor):
* Source/WebCore/svg/SVGImageElement.cpp:
(WebCore::SVGImageElement::insertedIntoAncestor):
* Source/WebCore/svg/SVGMPathElement.cpp:
(WebCore::SVGMPathElement::insertedIntoAncestor):
(WebCore::SVGMPathElement::didFinishInsertingNode):
* Source/WebCore/svg/SVGPathElement.cpp:
(WebCore::SVGPathElement::insertedIntoAncestor):
* Source/WebCore/svg/SVGScriptElement.cpp:
(WebCore::SVGScriptElement::insertedIntoAncestor):
* Source/WebCore/svg/SVGTRefElement.cpp:
(WebCore::SVGTRefElement::insertedIntoAncestor):
(WebCore::SVGTRefElement::didFinishInsertingNode):
* Source/WebCore/svg/SVGTextPathElement.cpp:
(WebCore::SVGTextPathElement::didFinishInsertingNode):
* Source/WebCore/svg/SVGTitleElement.cpp:
(WebCore::SVGTitleElement::insertedIntoAncestor):
* Source/WebCore/svg/SVGUseElement.cpp:
(WebCore::SVGUseElement::insertedIntoAncestor):
(WebCore::SVGUseElement::didFinishInsertingNode):
* Source/WebCore/svg/animation/SVGSMILElement.cpp:
(WebCore::SVGSMILElement::didFinishInsertingNode):

Canonical link: https://commits.webkit.org/264085@main
  • Loading branch information
rniwa committed May 15, 2023
1 parent 034442d commit a6a7aa5
Show file tree
Hide file tree
Showing 15 changed files with 51 additions and 16 deletions.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions Source/WebCore/svg/SVGFEImageElement.cpp
Expand Up @@ -150,6 +150,7 @@ Node::InsertedIntoAncestorResult SVGFEImageElement::insertedIntoAncestor(Inserti

void SVGFEImageElement::didFinishInsertingNode()
{
SVGFilterPrimitiveStandardAttributes::didFinishInsertingNode();
buildPendingResource();
}

Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/svg/SVGFontFaceElement.cpp
Expand Up @@ -300,15 +300,15 @@ void SVGFontFaceElement::rebuildFontFace()

Node::InsertedIntoAncestorResult SVGFontFaceElement::insertedIntoAncestor(InsertionType insertionType, ContainerNode& parentOfInsertedTree)
{
SVGElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
auto result = SVGElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
if (!insertionType.connectedToDocument) {
ASSERT(!m_fontElement);
return InsertedIntoAncestorResult::Done;
}
document().accessSVGExtensions().registerSVGFontFaceElement(*this);

rebuildFontFace();
return InsertedIntoAncestorResult::Done;
return result;
}

void SVGFontFaceElement::removedFromAncestor(RemovalType removalType, ContainerNode& oldParentOfRemovedTree)
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/svg/SVGImageElement.cpp
Expand Up @@ -184,13 +184,13 @@ void SVGImageElement::didAttachRenderers()

Node::InsertedIntoAncestorResult SVGImageElement::insertedIntoAncestor(InsertionType insertionType, ContainerNode& parentOfInsertedTree)
{
SVGGraphicsElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
auto result = SVGGraphicsElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
if (!insertionType.connectedToDocument)
return InsertedIntoAncestorResult::Done;
// Update image loader, as soon as we're living in the tree.
// We can only resolve base URIs properly, after that!
m_imageLoader.updateFromElement();
return InsertedIntoAncestorResult::Done;
return result;
}

const AtomString& SVGImageElement::imageSourceURL() const
Expand Down
5 changes: 3 additions & 2 deletions Source/WebCore/svg/SVGMPathElement.cpp
Expand Up @@ -79,14 +79,15 @@ void SVGMPathElement::clearResourceReferences()

Node::InsertedIntoAncestorResult SVGMPathElement::insertedIntoAncestor(InsertionType insertionType, ContainerNode& parentOfInsertedTree)
{
SVGElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
auto result = SVGElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
if (insertionType.connectedToDocument)
return InsertedIntoAncestorResult::NeedsPostInsertionCallback;
return InsertedIntoAncestorResult::Done;
return result;
}

void SVGMPathElement::didFinishInsertingNode()
{
SVGElement::didFinishInsertingNode();
buildPendingResource();
}

Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/svg/SVGPathElement.cpp
Expand Up @@ -95,9 +95,9 @@ void SVGPathElement::invalidateMPathDependencies()

Node::InsertedIntoAncestorResult SVGPathElement::insertedIntoAncestor(InsertionType insertionType, ContainerNode& parentOfInsertedTree)
{
SVGGeometryElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
auto result = SVGGeometryElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
invalidateMPathDependencies();
return InsertedIntoAncestorResult::Done;
return result;
}

void SVGPathElement::removedFromAncestor(RemovalType removalType, ContainerNode& oldParentOfRemovedTree)
Expand Down
5 changes: 3 additions & 2 deletions Source/WebCore/svg/SVGScriptElement.cpp
Expand Up @@ -64,8 +64,9 @@ void SVGScriptElement::svgAttributeChanged(const QualifiedName& attrName)

Node::InsertedIntoAncestorResult SVGScriptElement::insertedIntoAncestor(InsertionType insertionType, ContainerNode& parentOfInsertedTree)
{
SVGElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
return ScriptElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
auto result1 = SVGElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
auto result2 = ScriptElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
return result1 == InsertedIntoAncestorResult::NeedsPostInsertionCallback ? result1 : result2;
}

void SVGScriptElement::didFinishInsertingNode()
Expand Down
5 changes: 3 additions & 2 deletions Source/WebCore/svg/SVGTRefElement.cpp
Expand Up @@ -248,14 +248,15 @@ void SVGTRefElement::buildPendingResource()

Node::InsertedIntoAncestorResult SVGTRefElement::insertedIntoAncestor(InsertionType insertionType, ContainerNode& parentOfInsertedTree)
{
SVGElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
auto result = SVGElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
if (insertionType.connectedToDocument)
return InsertedIntoAncestorResult::NeedsPostInsertionCallback;
return InsertedIntoAncestorResult::Done;
return result;
}

void SVGTRefElement::didFinishInsertingNode()
{
SVGTextPositioningElement::didFinishInsertingNode();
buildPendingResource();
}

Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/svg/SVGTextPathElement.cpp
Expand Up @@ -168,6 +168,7 @@ Node::InsertedIntoAncestorResult SVGTextPathElement::insertedIntoAncestor(Insert

void SVGTextPathElement::didFinishInsertingNode()
{
SVGTextContentElement::buildPendingResource();
buildPendingResource();
}

Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/svg/SVGTitleElement.cpp
Expand Up @@ -43,9 +43,9 @@ Ref<SVGTitleElement> SVGTitleElement::create(const QualifiedName& tagName, Docum

Node::InsertedIntoAncestorResult SVGTitleElement::insertedIntoAncestor(InsertionType insertionType, ContainerNode& parentOfInsertedTree)
{
SVGElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
auto result = SVGElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
document().titleElementAdded(*this);
return InsertedIntoAncestorResult::Done;
return result;
}

void SVGTitleElement::removedFromAncestor(RemovalType removalType, ContainerNode& oldParentOfRemovedTree)
Expand Down
5 changes: 3 additions & 2 deletions Source/WebCore/svg/SVGUseElement.cpp
Expand Up @@ -109,19 +109,20 @@ void SVGUseElement::attributeChanged(const QualifiedName& name, const AtomString

Node::InsertedIntoAncestorResult SVGUseElement::insertedIntoAncestor(InsertionType insertionType, ContainerNode& parentOfInsertedTree)
{
SVGGraphicsElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
auto result = SVGGraphicsElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
if (insertionType.connectedToDocument) {
if (m_shadowTreeNeedsUpdate)
document().addElementWithPendingUserAgentShadowTreeUpdate(*this);
invalidateShadowTree();
// FIXME: Move back the call to updateExternalDocument() here once notifyFinished is made always async.
return InsertedIntoAncestorResult::NeedsPostInsertionCallback;
}
return InsertedIntoAncestorResult::Done;
return result;
}

void SVGUseElement::didFinishInsertingNode()
{
SVGGraphicsElement::didFinishInsertingNode();
updateExternalDocument();
}

Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/svg/animation/SVGSMILElement.cpp
Expand Up @@ -291,6 +291,7 @@ Node::InsertedIntoAncestorResult SVGSMILElement::insertedIntoAncestor(InsertionT

void SVGSMILElement::didFinishInsertingNode()
{
SVGElement::didFinishInsertingNode();
buildPendingResource();
}

Expand Down

0 comments on commit a6a7aa5

Please sign in to comment.