Bug 50240 - Unable to indirectly animate visibility of SVGUseElement
Summary: Unable to indirectly animate visibility of SVGUseElement
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh Intel OS X 10.6
: P2 Normal
Assignee: Rob Buis
URL:
Keywords:
Depends on:
Blocks: 52630
  Show dependency treegraph
 
Reported: 2010-11-30 07:43 PST by ken
Modified: 2011-01-18 08:55 PST (History)
6 users (show)

See Also:


Attachments
Broken behavior testcase, behaves the same bad way also on Windows platform... (15.02 KB, image/svg+xml)
2010-12-05 04:37 PST, marek
no flags Details
Patch (6.28 KB, patch)
2011-01-13 00:52 PST, Leo Yang
zimmermann: review-
Details | Formatted Diff | Diff
Revised patch version 2 (5.28 KB, patch)
2011-01-16 18:59 PST, Leo Yang
zimmermann: review-
Details | Formatted Diff | Diff
Revised patch version 3 (5.28 KB, patch)
2011-01-17 18:27 PST, Leo Yang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description ken 2010-11-30 07:43:00 PST
When viewing the SVG below, the red rectangle should disappear at 3s.

The red rectangle is referenced by a <use>.
The <use> is contained inside a <g>. 
The visibility of the <g> is set to hidden at 3s.

<svg width="100%" height="100%" viewBox="0 0 1000 800"
  xmlns="http://www.w3.org/2000/svg"
  xmlns:xlink="http://www.w3.org/1999/xlink">
  <defs>
    <rect id="r" x="100" y="100" width="800" height="600" fill="red" stroke="none"/>
  </defs>
  <g>
    <use xlink:href="#r"/>
    <set attributeType="XML" attributeName="visibility" to="hidden" begin="3s"/>
  </g>
</svg>

Works with Safari 4.0.5 (531.22.7), Windows XP
Doesn't work with
  Chrome 7.0.517.44, Windows XP
  Safari 5.0.3 (6533.19.4), Mac OS X 10.6

A subsequent browser Zoom causes the red rectangle to disappear.

The <set> animation does work when directly targeting the <use>

  <use xlink:href="#r">
    <set attributeType="XML" attributeName="visibility" to="hidden" begin="3s"/>
  </use>
Comment 1 marek 2010-12-05 04:36:46 PST
Yes, I noticed the same buggy behavior in my game http://svg.kvalitne.cz/submarine/index.htm, which makes it unplayable at all.. :-(
I'm enclosing similar testcase extracted from my game...
Comment 2 marek 2010-12-05 04:37:44 PST
Created attachment 75627 [details]
Broken behavior testcase, behaves the same bad way also on Windows platform...
Comment 3 Leo Yang 2011-01-13 00:52:40 PST
Created attachment 78781 [details]
Patch
Comment 4 Nikolas Zimmermann 2011-01-13 15:24:46 PST
Comment on attachment 78781 [details]
Patch

Excellent catch! Code r+, Test r->
It's a pity that we need platform dependant results, maybe you could come up with a testcase that avoids font-size changes?
Can't you trigger the bug with parent fill='..' changes, or sth. else like stroke-width?
Comment 5 Leo Yang 2011-01-16 18:59:03 PST
Created attachment 79120 [details]
Revised patch version 2

Using fill = "..." is a good idea. Maybe we don't need platform dependent test case because dumRenderTree can produce filling color in text.
Comment 6 Nikolas Zimmermann 2011-01-17 08:16:25 PST
Comment on attachment 79120 [details]
Revised patch version 2

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

Excellent, almost r+. Our common style is to indidicate a "green rect" as success in the expected.png/txt files. Yours is red, please change the transition from black -> red, to red -> green, then I'll r+/cq+ it. Thanks for your patience :-)

> LayoutTests/svg/custom/use-inherit-style.svg:8
> +    <rect id="rect" x="0" y="0" width="100" height="60" />

Here you should use fill="red" as initial value...

> LayoutTests/svg/custom/use-inherit-style.svg:17
> +        document.getElementById("g").setAttribute("fill", "red");

... and change it to fill="green" here.
Comment 7 Leo Yang 2011-01-17 18:27:04 PST
Created attachment 79235 [details]
Revised patch version 3

We can't set initial fill value for the rectangle because otherwise the rectangle will not inherit fill property of <g> element. Using "green" causes 0x008000 color value which isn't consistent with the existing truly green png, so 0x00FF00 is used explicitly.
Comment 8 Nikolas Zimmermann 2011-01-18 07:29:59 PST
(In reply to comment #7)
> Created an attachment (id=79235) [details]
> Revised patch version 3
> 
> We can't set initial fill value for the rectangle because otherwise the rectangle will not inherit fill property of <g> element. Using "green" causes 0x008000 color value which isn't consistent with the existing truly green png, so 0x00FF00 is used explicitly.

I'm sorry, with 'truly green' I meant the "green" color value, 0x008000. Using "green" is perfectly fine.
Comment 9 Nikolas Zimmermann 2011-01-18 07:30:52 PST
Comment on attachment 79235 [details]
Revised patch version 3

I don't care though whether the rect is 00FF00 or just 00FF00, either is fine, so r=me.
Comment 10 WebKit Commit Bot 2011-01-18 07:57:16 PST
Comment on attachment 79235 [details]
Revised patch version 3

Clearing flags on attachment: 79235

Committed r76027: <http://trac.webkit.org/changeset/76027>
Comment 11 WebKit Commit Bot 2011-01-18 07:57:21 PST
All reviewed patches have been landed.  Closing bug.