[webkit-reviews] review granted: [Bug 10916] Add tests to dump textual representations of entire HTML and SVG JS DOM trees : [Attachment 10704] Better version of the tests (actually includes results too)

[bugzilla-request-daemon at macosforge.org]

Geoffrey Garen <ggaren at apple.com> has granted Geoffrey Garen
<ggaren at apple.com>'s request for review:
Bug 10916: Add tests to dump textual representations of entire HTML and SVG JS
DOM trees
http://bugs.webkit.org/show_bug.cgi?id=10916

Attachment 10704: Better version of the tests (actually includes results too)
http://bugs.webkit.org/attachment.cgi?id=10704&action=edit

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
My main comment is that having a test like this is great, but I don't know if
it should be part of the regular layout tests. Unlike the table tests, it only
tells us if we've completely broken access to a certain property, not if the
property works or not. Since it tells us what the DOM is, rather than whether
the DOM works, this test seems more like a tool we should use periodically than
a layout test.

Comments about the code:

You probably know this, but the test still doesn't cover things like events,
collections, selection, and parts of the CSS dom.

This output is a little confusing:

+a.__proto__(HTMLAnchorElement).__proto__(HTMLElement) : object (HTMLElement)

Is 'a' the HTMLAnchorElement, or a.__proto__? What does the colon mean? How
does "(HTMLElement)" differ from "object (HTMLElement)?"

Can't you replace isPropertyDefinedOnPrototype was just using the "in" operator
on the prototype or testing for the existence of proptotype[property]? If
you're trying to optimize by skipping to the Object prototype first, (a) I'd be
surprised to learn that doing so was more efficient than a lookup inside the
engine; (b) why not just test the Object prototype first, and then use the
prototype chain?

PrintProperties shoud be "printProperties," since it's not a constructor. Also,
it's odd to have a verb for an object's name. "propertyPrinter" is a noun
alternative.

r=me 

I think this test belongs in WebKitTools.