Skip to content

Commit

Permalink
Merge r221377 - The SVG fragment identifier is not respected if it is…
Browse files Browse the repository at this point in the history
… a part of an HTTP URL

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

Patch by Said Abou-Hallawa <sabouhallawa@apple.com> on 2017-08-30
Reviewed by Darin Adler.

Source/WebCore:

If an image is referenced more than once in a page and the URL to that
image is an HTTP URL, one CachedImage is created for all the renderers
even if the original URLs have different fragmentIdentifiers. In this
case the fragment will be removed from the request which is associated
with the shared CachedImage. This CachedImage creates an SVGImage with
a URL but without a fragmentIdentifier. So SVGImage::draw() does not call
FrameView::scrollToFragment() and therefore the viewport is not setup
correctly for displaying the SVG in this case.

The fix is to move the url from the SVGImage to SVGImageForContainer.
Because there is one SVGImageForContainer created for every renderer,
we can move the full URL there. The drawing of an SVGImage has to start
from the SVGImageForContainer::draw() because the SVGImage may not have
an intrinsic size and the SVGImageForContainer is the one which knows
the destination rectangle. So SVGImageForContainer can pass the full url
to SVGImage::drawForContainer() which can be used to scrollToFragment()
before calling SVGImage::draw().

For clarity and consistency, all setContainerSizeForRenderer() will be
changed to setContainerContext() and the pair SizeAndZoom will be replaced
by the struct ContainerContext.

Tests: http/tests/svg/svg-fragment-background.html
       http/tests/svg/svg-fragment-image.html

* css/CSSCursorImageValue.h:
* css/CSSImageSetValue.cpp:
(WebCore::CSSImageSetValue::fillImageSet):
* css/CSSImageSetValue.h:
* loader/cache/CachedImage.cpp:
(WebCore::CachedImage::didRemoveClient):
(WebCore::CachedImage::switchClientsToRevalidatedResource):
(WebCore::CachedImage::allClientsRemoved):
(WebCore::CachedImage::setContainerContextForClient):
(WebCore::CachedImage::clear):
(WebCore::CachedImage::createImage):
(WebCore::CachedImage::setContainerSizeForRenderer): Deleted.
* loader/cache/CachedImage.h:
* rendering/RenderBoxModelObject.cpp:
(WebCore::RenderBoxModelObject::calculateBackgroundImageGeometry const):
(WebCore::RenderBoxModelObject::paintNinePieceImage):
* rendering/RenderImage.cpp:
(WebCore::RenderImage::updateInnerContentRect):
(WebCore::RenderImage::repaintOrMarkForLayout):
* rendering/RenderImageResource.cpp:
(WebCore::RenderImageResource::setContainerContext):
(WebCore::RenderImageResource::setContainerSizeForRenderer): Deleted.
* rendering/RenderImageResource.h:
* rendering/RenderImageResourceStyleImage.cpp:
(WebCore::RenderImageResourceStyleImage::setContainerContext):
(WebCore::RenderImageResourceStyleImage::setContainerSizeForRenderer): Deleted.
* rendering/RenderImageResourceStyleImage.h:
* rendering/RenderListMarker.cpp:
(WebCore::RenderListMarker::updateContent):
* rendering/shapes/ShapeOutsideInfo.cpp:
(WebCore::ShapeOutsideInfo::createShapeForImage const):
* rendering/style/StyleCachedImage.cpp:
(WebCore::StyleCachedImage::imageURL):
(WebCore::StyleCachedImage::setContainerContextForRenderer):
(WebCore::StyleCachedImage::setContainerSizeForRenderer): Deleted.
* rendering/style/StyleCachedImage.h:
* rendering/style/StyleGeneratedImage.h:
* rendering/style/StyleImage.h:
* rendering/svg/RenderSVGImage.cpp:
(WebCore::RenderSVGImage::updateImageViewport):
* svg/SVGImageElement.h:
* svg/graphics/SVGImage.cpp:
(WebCore::SVGImage::SVGImage):
(WebCore::SVGImage::drawForContainer):
(WebCore::SVGImage::drawPatternForContainer):
(WebCore::SVGImage::draw):
(WebCore::SVGImage::dump const): Deleted.
* svg/graphics/SVGImage.h:
* svg/graphics/SVGImageCache.cpp:
(WebCore::SVGImageCache::setContainerContextForClient):
(WebCore::SVGImageCache::setContainerSizeForRenderer): Deleted.
* svg/graphics/SVGImageCache.h:
* svg/graphics/SVGImageForContainer.cpp:
(WebCore::SVGImageForContainer::size const):
(WebCore::SVGImageForContainer::draw):
(WebCore::SVGImageForContainer::drawPattern):
* svg/graphics/SVGImageForContainer.h:

LayoutTests:

* http/tests/svg/resources/rgb-icons-1.svg: Added.
* http/tests/svg/resources/rgb-icons-2.svg: Added.
* http/tests/svg/resources/rgb-icons-3.svg: Added.
* http/tests/svg/svg-fragment-background-expected.html: Added.
* http/tests/svg/svg-fragment-background.html: Added.
* http/tests/svg/svg-fragment-image-expected.html: Added.
* http/tests/svg/svg-fragment-image.html: Added.
  • Loading branch information
Said Abou-Hallawa authored and carlosgcampos committed Sep 2, 2017
1 parent 0cb983e commit ef69d92
Show file tree
Hide file tree
Showing 34 changed files with 431 additions and 120 deletions.
15 changes: 15 additions & 0 deletions LayoutTests/ChangeLog
@@ -1,3 +1,18 @@
2017-08-30 Said Abou-Hallawa <sabouhallawa@apple.com>

The SVG fragment identifier is not respected if it is a part of an HTTP URL
https://bugs.webkit.org/show_bug.cgi?id=163811

Reviewed by Darin Adler.

* http/tests/svg/resources/rgb-icons-1.svg: Added.
* http/tests/svg/resources/rgb-icons-2.svg: Added.
* http/tests/svg/resources/rgb-icons-3.svg: Added.
* http/tests/svg/svg-fragment-background-expected.html: Added.
* http/tests/svg/svg-fragment-background.html: Added.
* http/tests/svg/svg-fragment-image-expected.html: Added.
* http/tests/svg/svg-fragment-image.html: Added.

2017-08-29 Said Abou-Hallawa <sabouhallawa@apple.com>

Assertion failure when opening a file with a missing tag closing bracket
Expand Down
9 changes: 9 additions & 0 deletions LayoutTests/http/tests/svg/resources/rgb-icons-1.svg
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
5 changes: 5 additions & 0 deletions LayoutTests/http/tests/svg/resources/rgb-icons-2.svg
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
15 changes: 15 additions & 0 deletions LayoutTests/http/tests/svg/resources/rgb-icons-3.svg
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
21 changes: 21 additions & 0 deletions LayoutTests/http/tests/svg/svg-fragment-background-expected.html
@@ -0,0 +1,21 @@
<!DOCTYPE html>
<html>
<head>
<style>
span {
display: inline-block;
width: 100px;
height: 100px;
background-color: green;
}
</style>
</head>
<body>
<div>
<p>background-image with with hide/show embedded :target CSS</p>
<span></span>
<span></span>
<span></span>
</div>
</body>
</html>
37 changes: 37 additions & 0 deletions LayoutTests/http/tests/svg/svg-fragment-background.html
@@ -0,0 +1,37 @@
<!DOCTYPE html>
<html>
<head>
<style>
.icon {
display: inline-block;
width: 100px;
height: 100px;
}
.green-icon-1 {
background: url(resources/rgb-icons-3.svg#green-icon) no-repeat;
}
.green-icon-2 {
background: url(http://localhost:8000/svg/resources/rgb-icons-3.svg#green-icon) no-repeat;
}
</style>
</head>
<body>
<div id="group-1">
<p>background-image with with hide/show embedded :target CSS</p>
<span class="icon green-icon-1"></span>
<span class="icon green-icon-2"></span>
<span class="icon" id="green-icon"></span>
</div>
<script>
if (window.testRunner)
testRunner.waitUntilDone();

document.getElementById("green-icon").style.background = "url('http://127.0.0.1:8000/svg/resources/rgb-icons-3.svg#green-icon') no-repeat";

setTimeout(function() {
if (window.testRunner)
testRunner.notifyDone();
}, 50);
</script>
</body>
</html>
33 changes: 33 additions & 0 deletions LayoutTests/http/tests/svg/svg-fragment-image-expected.html
@@ -0,0 +1,33 @@
<!DOCTYPE html>
<html>
<head>
<style>
span {
display: inline-block;
width: 100px;
height: 100px;
background-color: green;
}
</style>
</head>
<body>
<div>
<p>&lt;img> with #fragment referencing &lt;view></p>
<span></span>
<span></span>
<span></span>
</div>
<div>
<p>&lt;img> with #svgView(viewBox())</p>
<span></span>
<span></span>
<span></span>
<div>
<div>
<p>&lt;img> with hide/show embedded :target CSS</p>
<span></span>
<span></span>
<span></span>
</div>
</body>
</html>
73 changes: 73 additions & 0 deletions LayoutTests/http/tests/svg/svg-fragment-image.html
@@ -0,0 +1,73 @@
<!DOCTYPE html>
<html>
<head>
<style>
img {
width: 100px;
height: 100px;
}
</style>
</head>
<body>
<div id="group-1">
<p>&lt;img> with #fragment referencing &lt;view></p>
<img src="resources/rgb-icons-1.svg#green-icon-view">
<img src="http://localhost:8000/svg/resources/rgb-icons-1.svg#green-icon-view">
</div>
<div id="group-2">
<p>&lt;img> with #svgView(viewBox())</p>
<img src="resources/rgb-icons-2.svg#svgView(viewBox(0,32,32,32))">
<img src="http://localhost:8000/svg/resources/rgb-icons-2.svg#svgView(viewBox(0,32,32,32))">
</div>
<div id="group-3">
<p>&lt;img> with hide/show embedded :target CSS</p>
<img src="resources/rgb-icons-3.svg#green-icon">
<img src="http://localhost:8000/svg/resources/rgb-icons-3.svg#green-icon">
</div>
<script>
function createAndLoadImage(parent, src, srcset) {
return new Promise((resolve) => {
var image = new Image;
parent.appendChild(image);
image.onload = (() => {
resolve(image);
});
image.src = src;
image.srcset = srcset;
});
}

(function() {
if (window.testRunner)
testRunner.waitUntilDone();

var imageData = [
{
"parent": document.getElementById("group-1"),
"src": "http://127.0.0.1:8000/svg/resources/rgb-icons-1.svg#red-icon-view",
"srcset": "http://127.0.0.1:8000/svg/resources/rgb-icons-1.svg#green-icon-view",
},
{
"parent": document.getElementById("group-2"),
"src": "http://127.0.0.1:8000/svg/resources/rgb-icons-2.svg#svgView(viewBox(0,0,32,32))",
"srcset": "http://127.0.0.1:8000/svg/resources/rgb-icons-2.svg#svgView(viewBox(0,32,32,32))"
},
{
"parent": document.getElementById("group-3"),
"src" : "http://127.0.0.1:8000/svg/resources/rgb-icons-3.svg#red-icon",
"srcset" : "http://127.0.0.1:8000/svg/resources/rgb-icons-3.svg#green-icon"
},
];

var promises = [];
for (let datum of imageData)
promises.push(createAndLoadImage(datum.parent, datum.src, datum.srcset));

Promise.all(promises).then(() => {
if (window.testRunner)
testRunner.notifyDone();
});
})();
</script>
</body>
</html>
90 changes: 90 additions & 0 deletions Source/WebCore/ChangeLog
@@ -1,3 +1,93 @@
2017-08-30 Said Abou-Hallawa <sabouhallawa@apple.com>

The SVG fragment identifier is not respected if it is a part of an HTTP URL
https://bugs.webkit.org/show_bug.cgi?id=163811

Reviewed by Darin Adler.

If an image is referenced more than once in a page and the URL to that
image is an HTTP URL, one CachedImage is created for all the renderers
even if the original URLs have different fragmentIdentifiers. In this
case the fragment will be removed from the request which is associated
with the shared CachedImage. This CachedImage creates an SVGImage with
a URL but without a fragmentIdentifier. So SVGImage::draw() does not call
FrameView::scrollToFragment() and therefore the viewport is not setup
correctly for displaying the SVG in this case.

The fix is to move the url from the SVGImage to SVGImageForContainer.
Because there is one SVGImageForContainer created for every renderer,
we can move the full URL there. The drawing of an SVGImage has to start
from the SVGImageForContainer::draw() because the SVGImage may not have
an intrinsic size and the SVGImageForContainer is the one which knows
the destination rectangle. So SVGImageForContainer can pass the full url
to SVGImage::drawForContainer() which can be used to scrollToFragment()
before calling SVGImage::draw().

For clarity and consistency, all setContainerSizeForRenderer() will be
changed to setContainerContext() and the pair SizeAndZoom will be replaced
by the struct ContainerContext.

Tests: http/tests/svg/svg-fragment-background.html
http/tests/svg/svg-fragment-image.html

* css/CSSCursorImageValue.h:
* css/CSSImageSetValue.cpp:
(WebCore::CSSImageSetValue::fillImageSet):
* css/CSSImageSetValue.h:
* loader/cache/CachedImage.cpp:
(WebCore::CachedImage::didRemoveClient):
(WebCore::CachedImage::switchClientsToRevalidatedResource):
(WebCore::CachedImage::allClientsRemoved):
(WebCore::CachedImage::setContainerContextForClient):
(WebCore::CachedImage::clear):
(WebCore::CachedImage::createImage):
(WebCore::CachedImage::setContainerSizeForRenderer): Deleted.
* loader/cache/CachedImage.h:
* rendering/RenderBoxModelObject.cpp:
(WebCore::RenderBoxModelObject::calculateBackgroundImageGeometry const):
(WebCore::RenderBoxModelObject::paintNinePieceImage):
* rendering/RenderImage.cpp:
(WebCore::RenderImage::updateInnerContentRect):
(WebCore::RenderImage::repaintOrMarkForLayout):
* rendering/RenderImageResource.cpp:
(WebCore::RenderImageResource::setContainerContext):
(WebCore::RenderImageResource::setContainerSizeForRenderer): Deleted.
* rendering/RenderImageResource.h:
* rendering/RenderImageResourceStyleImage.cpp:
(WebCore::RenderImageResourceStyleImage::setContainerContext):
(WebCore::RenderImageResourceStyleImage::setContainerSizeForRenderer): Deleted.
* rendering/RenderImageResourceStyleImage.h:
* rendering/RenderListMarker.cpp:
(WebCore::RenderListMarker::updateContent):
* rendering/shapes/ShapeOutsideInfo.cpp:
(WebCore::ShapeOutsideInfo::createShapeForImage const):
* rendering/style/StyleCachedImage.cpp:
(WebCore::StyleCachedImage::imageURL):
(WebCore::StyleCachedImage::setContainerContextForRenderer):
(WebCore::StyleCachedImage::setContainerSizeForRenderer): Deleted.
* rendering/style/StyleCachedImage.h:
* rendering/style/StyleGeneratedImage.h:
* rendering/style/StyleImage.h:
* rendering/svg/RenderSVGImage.cpp:
(WebCore::RenderSVGImage::updateImageViewport):
* svg/SVGImageElement.h:
* svg/graphics/SVGImage.cpp:
(WebCore::SVGImage::SVGImage):
(WebCore::SVGImage::drawForContainer):
(WebCore::SVGImage::drawPatternForContainer):
(WebCore::SVGImage::draw):
(WebCore::SVGImage::dump const): Deleted.
* svg/graphics/SVGImage.h:
* svg/graphics/SVGImageCache.cpp:
(WebCore::SVGImageCache::setContainerContextForClient):
(WebCore::SVGImageCache::setContainerSizeForRenderer): Deleted.
* svg/graphics/SVGImageCache.h:
* svg/graphics/SVGImageForContainer.cpp:
(WebCore::SVGImageForContainer::size const):
(WebCore::SVGImageForContainer::draw):
(WebCore::SVGImageForContainer::drawPattern):
* svg/graphics/SVGImageForContainer.h:

2017-08-29 Said Abou-Hallawa <sabouhallawa@apple.com>

Assertion failure when opening a file with a missing tag closing bracket
Expand Down
2 changes: 2 additions & 0 deletions Source/WebCore/css/CSSCursorImageValue.h
Expand Up @@ -52,6 +52,8 @@ class CSSCursorImageValue final : public CSSValue {
return IntPoint(-1, -1);
}

const URL& imageURL() const { return m_originalURL; }

String customCSSText() const;

std::pair<CachedImage*, float> loadImage(CachedResourceLoader&, const ResourceLoaderOptions&);
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/css/CSSImageSetValue.cpp
Expand Up @@ -53,7 +53,7 @@ void CSSImageSetValue::fillImageSet()
size_t i = 0;
while (i < length) {
CSSValue* imageValue = item(i);
String imageURL = downcast<CSSImageValue>(*imageValue).url();
URL imageURL = downcast<CSSImageValue>(*imageValue).url();

++i;
ASSERT_WITH_SECURITY_IMPLICATION(i < length);
Expand Down
4 changes: 3 additions & 1 deletion Source/WebCore/css/CSSImageSetValue.h
Expand Up @@ -50,14 +50,16 @@ class CSSImageSetValue final : public CSSValueList {
String customCSSText() const;

struct ImageWithScale {
String imageURL;
URL imageURL;
float scaleFactor;
};

bool traverseSubresources(const WTF::Function<bool (const CachedResource&)>& handler) const;

void updateDeviceScaleFactor(const Document&);

URL bestImageForScaleFactorURL() { return bestImageForScaleFactor().imageURL; }

protected:
ImageWithScale bestImageForScaleFactor();

Expand Down

0 comments on commit ef69d92

Please sign in to comment.