- Revision
- 222304
- Author
- s...@apple.com
- Date
- 2017-09-20 17:35:41 -0700 (Wed, 20 Sep 2017)
Log Message
REGRESSION(r191731): SVGPatternElement can only reference another SVGPatternElement in the same SVG document
https://bugs.webkit.org/show_bug.cgi?id=176221
Reviewed by Tim Horton.
Source/WebCore:
According to the specs:
https://www.w3.org/TR/SVG11/filters.html#FilterElementHrefAttribute
https://www.w3.org/TR/SVG11/pservers.html#LinearGradientElementHrefAttribute
https://www.w3.org/TR/SVG11/pservers.html#RadialGradientElementHrefAttribute
https://www.w3.org/TR/SVG11/pservers.html#PatternElementHrefAttribute
The xlink:href attribute of the SVG filter, gradient and pattern elements
must reference another element within the current SVG of the same type.
In r191731, the code of SVGPatternElement::collectPatternAttributes() was
removed and replaced by RenderSVGResourcePattern::collectPatternAttributes()
to avoid cyclic reference in the pattern element. The problem is the old
code used to check whether the referenced element is<SVGPatternElement>
before casting it. This code was not copied to the new function. So we
now allow the SVGPatternElement to reference any SVG resource element.
To fix this issue, we need to prevent SVGResources from chaining an incorrect
type of element to the SVG filter, gradient and pattern elements.
We also need to use the SVGResources for getting the referenced element
when collecting the attributes for the gradient elements. SVGResources solves
the cyclic referencing issue so there is no need to repeat the same code
in many places. Also, from now on the SVGResources will have valid linked
resource only. So casting the referenced element should always be valid.
Tests: svg/custom/pattern-invalid-content-inheritance.svg
* rendering/svg/RenderSVGResourcePattern.cpp:
(WebCore::RenderSVGResourcePattern::collectPatternAttributes const): Asserts
the linkedResource is of type RenderSVGResourcePattern.
* rendering/svg/SVGResources.cpp:
(WebCore::SVGResources::SVGResources):
(WebCore::isChainableResource): Ensure that an SVG resource can reference
only an SVG resource with the valid type.
(WebCore::SVGResources::buildCachedResources):
* rendering/svg/SVGResources.h:
LayoutTests:
* svg/custom/pattern-invalid-content-inheritance-expected.svg: Added.
* svg/custom/pattern-invalid-content-inheritance.svg: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (222303 => 222304)
--- trunk/LayoutTests/ChangeLog 2017-09-21 00:30:17 UTC (rev 222303)
+++ trunk/LayoutTests/ChangeLog 2017-09-21 00:35:41 UTC (rev 222304)
@@ -1,3 +1,13 @@
+2017-09-20 Said Abou-Hallawa <sabouhall...@apple.com>
+
+ REGRESSION(r191731): SVGPatternElement can only reference another SVGPatternElement in the same SVG document
+ https://bugs.webkit.org/show_bug.cgi?id=176221
+
+ Reviewed by Tim Horton.
+
+ * svg/custom/pattern-invalid-content-inheritance-expected.svg: Added.
+ * svg/custom/pattern-invalid-content-inheritance.svg: Added.
+
2017-09-20 Per Arne Vollan <pvol...@apple.com>
Mark transitions/transition-display-property.html as flaky on Windows.
Added: trunk/LayoutTests/svg/custom/pattern-invalid-content-inheritance-expected.svg (0 => 222304)
--- trunk/LayoutTests/svg/custom/pattern-invalid-content-inheritance-expected.svg (rev 0)
+++ trunk/LayoutTests/svg/custom/pattern-invalid-content-inheritance-expected.svg 2017-09-21 00:35:41 UTC (rev 222304)
@@ -0,0 +1,3 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+ <rect fill="green" x="10" y="10" width="100" height="100"/>
+</svg>
Added: trunk/LayoutTests/svg/custom/pattern-invalid-content-inheritance.svg (0 => 222304)
--- trunk/LayoutTests/svg/custom/pattern-invalid-content-inheritance.svg (rev 0)
+++ trunk/LayoutTests/svg/custom/pattern-invalid-content-inheritance.svg 2017-09-21 00:35:41 UTC (rev 222304)
@@ -0,0 +1,9 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+ <pattern id="pattern" height="100" width="100" patternUnits="userSpaceOnUse" xlink:href=""
+ <rect width="100" height="100" fill="green"/>
+ </pattern>
+ <filter id="filter" filterUnits="userSpaceOnUse" xlink:href=""
+ <feFlood x="120" y="10" width="100" height="100" flood-color="green"/>
+ </filter>
+ <rect fill="url(#pattern)" x="10" y="10" width="100" height="100"/>
+</svg>
Modified: trunk/Source/WebCore/ChangeLog (222303 => 222304)
--- trunk/Source/WebCore/ChangeLog 2017-09-21 00:30:17 UTC (rev 222303)
+++ trunk/Source/WebCore/ChangeLog 2017-09-21 00:35:41 UTC (rev 222304)
@@ -1,3 +1,48 @@
+2017-09-20 Said Abou-Hallawa <sabouhall...@apple.com>
+
+ REGRESSION(r191731): SVGPatternElement can only reference another SVGPatternElement in the same SVG document
+ https://bugs.webkit.org/show_bug.cgi?id=176221
+
+ Reviewed by Tim Horton.
+
+ According to the specs:
+
+ https://www.w3.org/TR/SVG11/filters.html#FilterElementHrefAttribute
+ https://www.w3.org/TR/SVG11/pservers.html#LinearGradientElementHrefAttribute
+ https://www.w3.org/TR/SVG11/pservers.html#RadialGradientElementHrefAttribute
+ https://www.w3.org/TR/SVG11/pservers.html#PatternElementHrefAttribute
+
+ The xlink:href attribute of the SVG filter, gradient and pattern elements
+ must reference another element within the current SVG of the same type.
+
+ In r191731, the code of SVGPatternElement::collectPatternAttributes() was
+ removed and replaced by RenderSVGResourcePattern::collectPatternAttributes()
+ to avoid cyclic reference in the pattern element. The problem is the old
+ code used to check whether the referenced element is<SVGPatternElement>
+ before casting it. This code was not copied to the new function. So we
+ now allow the SVGPatternElement to reference any SVG resource element.
+
+ To fix this issue, we need to prevent SVGResources from chaining an incorrect
+ type of element to the SVG filter, gradient and pattern elements.
+
+ We also need to use the SVGResources for getting the referenced element
+ when collecting the attributes for the gradient elements. SVGResources solves
+ the cyclic referencing issue so there is no need to repeat the same code
+ in many places. Also, from now on the SVGResources will have valid linked
+ resource only. So casting the referenced element should always be valid.
+
+ Tests: svg/custom/pattern-invalid-content-inheritance.svg
+
+ * rendering/svg/RenderSVGResourcePattern.cpp:
+ (WebCore::RenderSVGResourcePattern::collectPatternAttributes const): Asserts
+ the linkedResource is of type RenderSVGResourcePattern.
+ * rendering/svg/SVGResources.cpp:
+ (WebCore::SVGResources::SVGResources):
+ (WebCore::isChainableResource): Ensure that an SVG resource can reference
+ only an SVG resource with the valid type.
+ (WebCore::SVGResources::buildCachedResources):
+ * rendering/svg/SVGResources.h:
+
2017-09-20 Daniel Bates <daba...@apple.com>
Spelling and grammar dots should not overlap
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGResourcePattern.cpp (222303 => 222304)
--- trunk/Source/WebCore/rendering/svg/RenderSVGResourcePattern.cpp 2017-09-21 00:30:17 UTC (rev 222303)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGResourcePattern.cpp 2017-09-21 00:35:41 UTC (rev 222304)
@@ -64,6 +64,7 @@
pattern.collectPatternAttributes(attributes);
auto* resources = SVGResourcesCache::cachedResourcesForRenderer(*current);
+ ASSERT_IMPLIES(resources && resources->linkedResource(), is<RenderSVGResourcePattern>(resources->linkedResource()));
current = resources ? downcast<RenderSVGResourcePattern>(resources->linkedResource()) : nullptr;
}
}
Modified: trunk/Source/WebCore/rendering/svg/SVGResources.cpp (222303 => 222304)
--- trunk/Source/WebCore/rendering/svg/SVGResources.cpp 2017-09-21 00:30:17 UTC (rev 222303)
+++ trunk/Source/WebCore/rendering/svg/SVGResources.cpp 2017-09-21 00:35:41 UTC (rev 222304)
@@ -39,7 +39,6 @@
namespace WebCore {
SVGResources::SVGResources()
- : m_linkedResource(0)
{
}
@@ -154,6 +153,21 @@
return SVGURIReference::fragmentIdentifierFromIRIString(target, element.document());
}
+static inline bool isChainableResource(const SVGElement& element, const SVGElement& linkedResource)
+{
+ if (is<SVGPatternElement>(element))
+ return is<SVGPatternElement>(linkedResource);
+
+ if (is<SVGGradientElement>(element))
+ return is<SVGGradientElement>(linkedResource);
+
+ if (is<SVGFilterElement>(element))
+ return is<SVGFilterElement>(linkedResource);
+
+ ASSERT_NOT_REACHED();
+ return false;
+}
+
static inline RenderSVGResourceContainer* paintingResourceFromSVGPaint(Document& document, const SVGPaintType& paintType, const String& paintUri, AtomicString& id, bool& hasPendingResource)
{
if (paintType != SVG_PAINTTYPE_URI && paintType != SVG_PAINTTYPE_URI_RGBCOLOR && paintType != SVG_PAINTTYPE_URI_CURRENTCOLOR)
@@ -274,10 +288,13 @@
if (chainableResourceTags().contains(tagName)) {
AtomicString id(targetReferenceFromResource(element));
- if (setLinkedResource(getRenderSVGResourceContainerById(document, id)))
+ auto* linkedResource = getRenderSVGResourceContainerById(document, id);
+ if (!linkedResource)
+ registerPendingResource(extensions, id, element);
+ else if (isChainableResource(element, linkedResource->element())) {
+ setLinkedResource(linkedResource);
foundResources = true;
- else
- registerPendingResource(extensions, id, element);
+ }
}
return foundResources;
Modified: trunk/Source/WebCore/rendering/svg/SVGResources.h (222303 => 222304)
--- trunk/Source/WebCore/rendering/svg/SVGResources.h 2017-09-21 00:30:17 UTC (rev 222303)
+++ trunk/Source/WebCore/rendering/svg/SVGResources.h 2017-09-21 00:35:41 UTC (rev 222304)
@@ -155,7 +155,7 @@
std::unique_ptr<ClipperFilterMaskerData> m_clipperFilterMaskerData;
std::unique_ptr<MarkerData> m_markerData;
std::unique_ptr<FillStrokeData> m_fillStrokeData;
- RenderSVGResourceContainer* m_linkedResource;
+ RenderSVGResourceContainer* m_linkedResource { nullptr };
};
} // namespace WebCore
