[webkit-reviews] review denied: [Bug 42608] dumpAsMarkup test conversion: create-list-from-range-selection.html, create-list-with-hr.html, and insert-list-empty-div.html : [Attachment 62019] converts the tests

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jul 19 18:37:48 PDT 2010


Ojan Vafai <ojan at chromium.org> has denied Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 42608: dumpAsMarkup test conversion: create-list-from-range-selection.html,
create-list-with-hr.html, and insert-list-empty-div.html
https://bugs.webkit.org/show_bug.cgi?id=42608

Attachment 62019: converts the tests
https://bugs.webkit.org/attachment.cgi?id=62019&action=review

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
> Index: LayoutTests/resources/dump-as-markup.js
> ===================================================================
> --- LayoutTests/resources/dump-as-markup.js	(revision 63331)
> +++ LayoutTests/resources/dump-as-markup.js	(working copy)
> @@ -17,7 +17,7 @@ Markup.dump = function(opt_node)

> +Markup.setNode = function(node)

Can we call this setNodeToDump so it's more clear what it does? 

> +{
> +  if (typeof node == "string")
> +    node = document.getElementById(node);
> +  if (node instanceof Node)

I don't think you need this if-check. If node is not a Node here, we should
error out.

> +    Markup.node = node

This should be private, i.e., Markup._node.


More information about the webkit-reviews mailing list