Bug 15080 - <use> element not refreshed after adding objects by script in referrer
Summary: <use> element not refreshed after adding objects by script in referrer
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 523.x (Safari 3)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL: http://pilat.free.fr/english/svg_lib/...
Keywords: HasReduction
Depends on:
Blocks:
 
Reported: 2007-08-26 07:00 PDT by Michel Hirtzler
Modified: 2007-10-09 10:41 PDT (History)
0 users

See Also:


Attachments
Test case from Comment #2 (1.96 KB, image/svg+xml)
2007-09-24 21:49 PDT, David Kilzer (:ddkilzer)
no flags Details
First attempt (21.86 KB, patch)
2007-10-05 13:02 PDT, Rob Buis
darin: review-
Details | Formatted Diff | Diff
Updated patch. (21.86 KB, patch)
2007-10-06 00:09 PDT, Rob Buis
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.