Bug 15080

Summary: <use> element not refreshed after adding objects by script in referrer
Product: WebKit Reporter: Michel Hirtzler <pilat>
Component: SVGAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal Keywords: HasReduction
Priority: P2    
Version: 523.x (Safari 3)   
Hardware: All   
OS: All   
URL: http://pilat.free.fr/english/svg_lib/test_list_scroll.svg
Attachments:
Description Flags
Test case from Comment #2
none
First attempt
darin: review-
Updated patch. eric: review+

Description Michel Hirtzler 2007-08-26 07:00:32 PDT
This example is a drop down list. Content of list is added by script on opening widget. ( click on a rectangle to load widget )
With webkit list content is not show ...
I use clipPath and translate to show list in right place, but after some tests, reason come not from clipPath or translate.
I think that after objects will be added to referrer group by script, <use> element is not refresh ( I try without success to use forceRedraw() ... )
Michel Hirtzler
Very good job about svg, thanks
Comment 1 Eric Seidel (no email) 2007-09-24 08:59:47 PDT
Something is broken here.  I'm not sure if it's exactly as the reporter describes though.  We'll need a single-file reduction to really understand the problem.
Comment 2 Michel Hirtzler 2007-09-24 10:38:23 PDT
(In reply to comment #1)
> Something is broken here.  I'm not sure if it's exactly as the reporter
> describes though.  We'll need a single-file reduction to really understand the
> problem.

I post single file http://pilat.free.fr/Safari/use_bug.svg
By click new line is created in group and must appear in <use> referring to this group on right
This example run fine with Adobe plugin, Opera, Firefox and Batik .... but not in Safari (beta 3.03 or lasted nighty) new line is in group but not in <use> ...
Comment 3 David Kilzer (:ddkilzer) 2007-09-24 21:49:10 PDT
Created attachment 16379 [details]
Test case from Comment #2
Comment 4 Rob Buis 2007-10-05 13:02:22 PDT
Created attachment 16550 [details]
First attempt

This fixes it.
Cheers,

Rob.
Comment 5 Darin Adler 2007-10-05 14:42:21 PDT
Comment on attachment 16550 [details]
First attempt

+    if (ownerDocument()->parsing())
+        return;

Please use document(). ownerDocument() is just a slower way to call document() that returns 0 in the special case of a Document node.

Why is it OK to skip updateElementInstance if parsing is true? What guarantees it's called later?

review- for the ownerDocument thing and I'd also like to know the answer to my question.
Comment 6 Rob Buis 2007-10-06 00:06:45 PDT
Hi,

(In reply to comment #5)
> (From update of attachment 16550 [details] [edit])
> +    if (ownerDocument()->parsing())
> +        return;
> 
> Please use document(). ownerDocument() is just a slower way to call document()
> that returns 0 in the special case of a Document node.

Ok, will do. I'll also change the other spots where we use them, but in a separate patch.

> Why is it OK to skip updateElementInstance if parsing is true? What guarantees
> it's called later?

Since addChild is not used, every append triggers a childrenChanged while parsing. However, the related shadow tree is only built later when the <use>
is processed, and <use> needs a referenced tree that is fully build anyway. The only phase we didnt cover is dynamic addition and that is what this patch fixes. I hope that clears it up.
Cheers,

Rob.
Comment 7 Rob Buis 2007-10-06 00:09:41 PDT
Created attachment 16562 [details]
Updated patch.

Now uses document().
Cheers,

Rob.
Comment 8 Eric Seidel (no email) 2007-10-08 16:33:41 PDT
Comment on attachment 16562 [details]
Updated patch.

Looks good.  I wish that the copy could happen lazily (invalidate the ElementInstance and have it re-pull.)  I'll file a bug about that.
Comment 9 Eric Seidel (no email) 2007-10-08 17:39:48 PDT
Oh, one thought I had.  There are no valid targets of <use> which are not SVGStyledElements, right?
Comment 10 Rob Buis 2007-10-09 10:41:56 PDT
Landed in r26171.