Bug 118140

Summary: SVG relayout problem when displayed with different image box heights
Product: WebKit Reporter: Morten Stenshorne <mstensho>
Component: SVGAssignee: Morten Stenshorne <mstensho>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, esprehn+autocc, glenn, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
SVG helper file for TC
none
Test case
none
Patch none

Description Morten Stenshorne 2013-06-27 09:37:45 PDT
SVGs are not relaid out deeply enough when there are two images with the same SVG URL, and the only difference between the two images is the height.
Comment 1 Morten Stenshorne 2013-06-27 09:38:38 PDT
Created attachment 205612 [details]
SVG helper file for TC
Comment 2 Morten Stenshorne 2013-06-27 09:39:30 PDT
Created attachment 205613 [details]
Test case
Comment 3 Morten Stenshorne 2013-06-28 02:38:39 PDT
Created attachment 205682 [details]
Patch
Comment 4 Dirk Schulze 2013-06-28 08:01:36 PDT
Comment on attachment 205682 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=205682&action=review

> LayoutTests/svg/as-image/same-source-different-height-expected.html:9
> +    <img src="resources/circle.svg" type="image/svg+xml" style="width:100px; height:300px;">
> +    <img src="resources/circle.svg?different-url" type="image/svg+xml" style="width:100px; height:100px;">

This looks like a problem with URL parsing. To force a relayout for each RenderView seems like shooting with guns. On the other hand it looks like we just rely on the image url instead of the image dimension. What happens exactly? Is the layout wrong, or does it look pixelated?
Comment 5 Morten Stenshorne 2013-06-28 09:51:52 PDT
Can you reproduce the problem if you open the testcase? I've only tested in Linux, but I've tested both the GTK port, Chromium content shell and official Chrome. All fail.

If there are several IMG elements with the same SVG SRC attribute, there will be one shared SVGImage, and therefore a shared Page and FrameView, which has to be laid out over again every time we display the image with a different size.

The problem is that if it is only the height that's different, the SVG root isn't re-aligned within the viewport. This only happens if the URLs are identical. Hence the "?blah" thing in the ref. Width-only changes are handled properly, though, but one can say that's expected from a layout engine. Traditionally, width changes requires children to be laid out again, while height changes "normally" (plain static block layout with no percentage heights, that is) don't affect the layout of chilren.

My patch only forces re-layout of the SVG root element itself, not the children, and it only does so if the viewport size has changed.
Comment 6 Philip Rogers 2013-06-28 09:57:28 PDT
Comment on attachment 205682 [details]
Patch

r=me; nice find.
Comment 7 WebKit Commit Bot 2013-06-28 11:01:31 PDT
Comment on attachment 205682 [details]
Patch

Clearing flags on attachment: 205682

Committed r152178: <http://trac.webkit.org/changeset/152178>
Comment 8 WebKit Commit Bot 2013-06-28 11:01:34 PDT
All reviewed patches have been landed.  Closing bug.