[webkit-reviews] review canceled: [Bug 189499] [SVG] fragment-only url 'url(#fragment)' should be resolved against the current document with regardless to HTML <base> element : [Attachment 375779] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Aug 9 00:26:47 PDT 2019

Fujii Hironori <Hironori.Fujii at sony.com> has canceled Fujii Hironori
<Hironori.Fujii at sony.com>'s request for review:
Bug 189499: [SVG] fragment-only url 'url(#fragment)' should be resolved against
the current document with regardless to HTML <base> element

Attachment 375779: Patch


--- Comment #22 from Fujii Hironori <Hironori.Fujii at sony.com> ---
Comment on attachment 375779
  --> https://bugs.webkit.org/attachment.cgi?id=375779

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

>> Source/WebCore/ChangeLog:8
>> +
> Can you please add a comment explaining what you are fixing in this patch?
> Can you also mention that this patch is based on the Blink change
> Is there a reason for not porting the change in

Will fix.
StyleBuilderConverter::convertClipPath has a issue it doesn't work with
external SVG references.
However I will check it again.

>> Source/WebCore/ChangeLog:21
>> +	    (WebCore::SVGURIReference::fragmentIdentifierFromIRIString): Return
the fragemnt if the URL starts with '#'.
> fragemnt

Will fix.

>> LayoutTests/svg/animations/local-url-target-reference.html:18
>> +  <set attributeName="fill" xlink:href="#rect" to="green"
> You do not have to include the xlink namespace in the href attribute.

Will fix.

>> LayoutTests/svg/custom/linking-base-external-reference.xhtml:-18
>> -</html>
> What is the reason for removing this test?

This is a test case for the old spec.

>> LayoutTests/svg/custom/local-url-references-webkit.html:53
>> +</svg>
> Why "webkit" is included in the name of the file? Does it test specific
WebKit functionalities? I think we should not do that.

I added local-url-references-webkit.html for WebKit issue (Comment 17). But, I
will remove this.

>> LayoutTests/svg/custom/local-url-references.html:53
>> +</svg>
> What is the difference between this test and
local-url-references-webkit.html? Also this test and the other one are too long
to read. They both test almost all the SVG resources. Can you split them into
smaller ones?

I replaced <rect> with <polygon> to avoid Bug 200501.
Got it. I will split.

More information about the webkit-reviews mailing list