Bug 55750 – SVG <image> referenced by <use> is displayed incorrectly

• Component: SVG
• Status: RESOLVED FIXED

[Cosmin Truta]

This is a Chromium-specific bug. The original test case runs well on Safari and on the other non-Chromium browsers.
Here is the original bug report:
http://code.google.com/p/chromium/issues/detail?id=42465

Underneath, however, all platforms need fixing.

The difference in behavior between Chromium and the other browsers is caused by the difference between KURL and KURLGoogle. The two URL implementation classes are slightly different in their handling of invalid URLs. However, in principle, this difference should not cause issues.

The KURL constructor takes the arguments (base, relative). When base is the empty string, and relative has no scheme (i.e. base="", relative="foo.svg"), the resulting KURL object is invalid. Both KURL and KURLGoogle agree on this.
The difference is that the KURL string (upon failure) is set to the relative part, while the KURLGoogle string (also upon failure) is set to the empty string:

base = ""
relative = "foo.svg"
----
KURL(base, relative).isValid() == false
KURL(base, relative).string() = "foo.svg"
----
KURLGooglePrivate(base, relative).isValid() == false
KURLGooglePrivate(base, relative).string() = ""

Neither of the two URL implementations is "right" or "wrong", it's just that KURL happens to work with this test case, while KURLGoogle, unluckily, doesn't.
The real problem here, which needs fixing, is that the resulting URL should not be invalid. The test case is perfectly legitimate.

The fix consists in setting the base part to the correct value, which is the URL of the document that contains the <image> element. This will lead to:
base = "scheme://path/to/base.svg"
relative = "foo.svg"
-----
KURL(base, relative).isValid() == KURLGooglePrivate(base, relative).isValid() == true
KURL(base, relative).string() == KURLGooglePrivate(base, relative).string() == "scheme://path/to/foo.svg"

[Cosmin Truta]

I opened this bug, although the bug 33971 already exists. I could not change the title of that bug, which, among other things, is littered with red herrings of various kinds.
Can someone with the applicable permissions please mark the other bug as a duplicate of this one?

[Cosmin Truta]

Created attachment 84704 [details]
Patch

[Adam Barth]

Comment on attachment 84704 [details]
Patch

looks right to me.  The call to stripLeadingAndTrailingHTMLSpaces probably isn't correct, but that's another issue.

[Fady Samuel]

*** Bug 33971 has been marked as a duplicate of this bug. ***

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 84704 [details]:

http/tests/media/video-load-twice.html bug 51138 (authors: [e-mail] and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 84704 [details]
Patch

Clearing flags on attachment: 84704

Committed r80516: <http://trac.webkit.org/changeset/80516>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/80517 might have broken GTK Linux 32-bit Release

[Nikolas Zimmermann]

This broke LayoutTests/svg/W3C-SVG-1.1/struct-image-07-t.svg. I'm wondering why this hasn't been noticed by the Chromium folks? Is the test skipped on all platforms?

[Nikolas Zimmermann]

Ping! struct-image-07-t pixel test remains broken, any news?

[Rob Buis]

Created attachment 87546 [details]
Patch

[Dirk Schulze]

Comment on attachment 87546 [details]
Patch

We just have one pixel test failing. Can you create DRT test for this?

[Rob Buis]

Created attachment 87747 [details]
Patch

[Nikolas Zimmermann]

Comment on attachment 87747 [details]
Patch

Looks great, r=me.

[Rob Buis]

Landed in r82689.