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

[bugzilla-daemon at webkit.org]

Said Abou-Hallawa <sabouhallawa at apple.com> has granted 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
https://bugs.webkit.org/show_bug.cgi?id=189499

Attachment 377548: Patch

https://bugs.webkit.org/attachment.cgi?id=377548&action=review




--- Comment #30 from Said Abou-Hallawa <sabouhallawa at apple.com> ---
Comment on attachment 377548
  --> https://bugs.webkit.org/attachment.cgi?id=377548
Patch

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

> LayoutTests/css3/filters/effect-reference-local-url-with-base.html:16
> +</style>
> +<svg>
> +  <filter id="filter" x="0" y="0" width="1" height="1"
color-interpolation-filters="sRGB">
> +    <feFlood flood-color="green"/>
> +  </filter>
> +</svg>
> +<div id="target"></div>

If you swap the order of the <svg> and the <div> elements, the expected file
will not have to have the mysterious  "<svg></svg>".

> LayoutTests/svg/custom/local-url-reference-clip-path.html:3
> +<svg width="400" height="300">

No need to set a size for the svg. The default size (300x15) should be
sufficient for the drawing.

> LayoutTests/svg/custom/local-url-reference-fill-expected.html:4
> +<svg width="400" height="300">
> +    <rect width="100" height="100" fill="green"/>
> +</svg>

Like the other expected files, this can be replaced by:

<div style="width: 100px; height: 100px; background-color: green"></div>

> LayoutTests/svg/custom/local-url-reference-fill.html:3
> +<svg width="400" height="300">

Ditto.

> LayoutTests/svg/custom/local-url-reference-fill.html:7
> +    <linearGradient id="inheritedPaint" href="#paint"/>

inheritedPaint is defined but not used. Did you mean to add something like
this:

    <g transform="translate(100)">
      <rect width="100" height="100" fill="url(#inheritedPaint) red"/>
    </g>

Or maybe you wanted to use it instead of #paint.

> LayoutTests/svg/custom/local-url-reference-filter-expected.html:4
> +<svg width="400" height="300">
> +    <rect width="100" height="100" fill="green"/>
> +</svg>

Ditto.

> LayoutTests/svg/custom/local-url-reference-stroke.html:7
> +    <linearGradient id="inheritedPaint" href="#paint"/>

inheritedPaint is defined but it is not used.