[webkit-reviews] review denied: [Bug 26117] REGRESSION (r37381-r37442) : Reproducible crash viewing an SVG : [Attachment 39980] Modified test case

[bugzilla-daemon at webkit.org]

Nikolas Zimmermann <zimmermann at kde.org> has denied Robin Qiu
<robin.qiu at torchmobile.com.cn>'s request for review:
Bug 26117: REGRESSION (r37381-r37442) : Reproducible crash viewing an SVG
https://bugs.webkit.org/show_bug.cgi?id=26117

Attachment 39980: Modified test case
https://bugs.webkit.org/attachment.cgi?id=39980&action=review

------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
Robin, thanks for getting started on fixing that bug.
Though the approach is not correct, I do wonder if you re-ran layout tests? No
new tests failing?

The SVGElementInstance tree should be a 1:1 map of the normalized, referenced
tree (aka. you expand the use elements to the content that actually get
rendered -- our shadow tree).
I agree that the current behaviour is wrong, leading to the second wrong
SVGCircleElement instance, though your new approach can't be right as well -
where is the <set> element?
The SVGSetElement has to show up in the SVGElmeentInstance tree as child of the
SVGCircleElement, that's not the case at the moment.

I suggest you look deeper - enable DUMP_INSTANCE_TREE and DUMP_SHADOW_TREE, and
try to find out the real cause why this gets out of sync.
That said, r- for the patch, as the handleDeepUseReferencing case has to stay
(see the Spec comment! :-).

If you need any further advice, feel free to contact me in private.