[Webkit-unassigned] [Bug 41655] Implement SVGSVGElement.getElementById

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=41655


Eric Seidel <eric at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #60923|review?                     |review-
               Flag|                            |




--- Comment #16 from Eric Seidel <eric at webkit.org>  2010-07-08 12:13:21 PST ---
(From update of attachment 60923)
WebCore/svg/SVGSVGElement.cpp:600
 +      if (element && element->isSVGElement() && element->isDescendantOf(this))
Why isSVGElement()?  Please test that case by adding some foreignObject content (or simply non-SVG content) under the SVG tree.

WebCore/svg/SVGSVGElement.cpp:606
 +          if (node->isElementNode()) {
This should use the "early return" style, with an early continue here.

LayoutTests/svg/custom/svg-getelementid.xhtml:3
 +      <div id="foo">
This does not test "foo" both before and after, like suggested in my comment.

LayoutTests/svg/custom/svg-getelementid-expected.txt:1
 +  PASS
If you're going to go to all the trouble to re-write the test, you might as well use the script-test js files (even if you don't make this a script test).  Teh output is much much much nicer to read.

LayoutTests/svg/custom/svg-getelementid.xhtml:4
 +        <div id="bar">
Why put all the content inside these divs instead of having them before or after the SVG content?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.