[webkit-reviews] review granted: [Bug 43286] Remove setNodeToDump from dump-as-markup.js : [Attachment 63271] Fixed per Ojan's comment. Asked him to take a look again

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Aug 2 15:45:47 PDT 2010


Ojan Vafai <ojan at chromium.org> has granted Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 43286: Remove setNodeToDump from dump-as-markup.js
https://bugs.webkit.org/show_bug.cgi?id=43286

Attachment 63271: Fixed per Ojan's comment. Asked him to take a look again
https://bugs.webkit.org/attachment.cgi?id=63271&action=review

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
> +    if (Markup._dumpCalls == 2 && Markup._firstCallDidNotHaveDescription) {
> +	   var wrapper = document.getElementById('dump-as-markup-1');
> +	   wrapper.insertBefore(document.createTextNode('\nDump of markup
1:\n'), wrapper.firstChild);
> +    }

> -    Markup._container.appendChild(document.createTextNode(markup));
> +    if (Markup._test_description && Markup._dumpCalls == 1)
> +	  
Markup._container.appendChild(document.createTextNode(Markup._test_description
+ '\n'))
> +
> +    var wrapper = document.createElement('span');
> +    wrapper.setAttribute('id', 'dump-as-markup-' + Markup._dumpCalls);

Nit: No need to use setAttribute with IDs. Can just do:
wrapper.id = 'dump-as-markup-' + Markup._dumpCalls;

Also, we could use classes instead of IDs. Then you can get the first one by
calling document.getElementsByClassName(class)[0].


More information about the webkit-reviews mailing list