Now that noAutoDump is added, setNodeToDump is redundant. We should make dump a little smarter not to print out the optional description, and remove setNodeToDump.
Created attachment 63134 [details] Patch
Comment on attachment 63134 [details] Patch Just a few nits. > +++ LayoutTests/resources/dump-as-markup.js (working copy) > - // If dump is not called by notifyDone, then print out optional description > - // because this test is manually calling dump. > - if (!Markup._done || opt_description) { > + if (Markup._dumpCalls > 1 || opt_description) { > if (!opt_description) > opt_description = "Dump of markup " + Markup._dumpCalls > if (Markup._dumpCalls > 1) > - markup += '\n'; > + markup += '\n' Missing semi-colon. > markup += '\n' + opt_description + ':\n'; > + } else { > + Markup._firstCallDidNotHaveDescription = true; > } Single-line else statement should not use brackets. > + if (Markup._dumpCalls > 1 && Markup._firstCallDidNotHaveDescription) { > + Markup._container.insertBefore( > + document.createTextNode('\nDump of markup 1:\n'), > + Markup._container.firstChild.nextSibling); This looks to me like it will only work on tests that have descriptions, no? How about wrapping the first dump in a span with an ID? Then you can getElementById instead of relying on the DOM structure. Or you could give each dump a classname and get do getElementsByClassName(class)[0]. Relying on the DOM structure is too fragile I think.
Created attachment 63271 [details] Fixed per Ojan's comment. Asked him to take a look again
Comment on attachment 63271 [details] Fixed per Ojan's comment. Asked him to take a look again > + 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].
Committed r64507: <http://trac.webkit.org/changeset/64507>
http://trac.webkit.org/changeset/64507 might have broken GTK Linux 64-bit Debug The following changes are on the blame list: http://trac.webkit.org/changeset/64506 http://trac.webkit.org/changeset/64507