Bug 26501

Summary: layoutTestController should have dumpAsMarkup
Product: WebKit Reporter: Ojan Vafai <ojan>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Severity: Normal CC: abarth, ap, aroben, eric, hamaji, jmalonzo, jparent, levin, mrowe, rniwa, tonikitoo, tony, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Macintosh   
OS: OS X 10.5   
Description Flags
initial prototype
Patch none

Description Ojan Vafai 2009-06-17 17:08:42 PDT
This is for tests that care about the resultant DOM, not the rendering of the DOM. Note that these tests would *not* do pixel comparisons, so they have many of the advantages of dumpAsText tests. This is essentially sugar for a dumpAsText tests that spits out the body's innerHTML. 

Although, I wonder if it should also include the markup for the HTML and HEAD elements. Seems like that would be good.

Also, it should somehow print the page's selection if there is one. Here's the IRC discussion around that:
othermaciej: ojan: maybe there is a good way for the page itself to print the selection range, if there is a good way to do so
ojan: othermaciej: i see two ways this could work
ojan: 1. after the markup dump, print the baseNode, baseOffset, etc like we do currently with the renderdumps
ojan: 2. actually include something like [START_SELECTION] inline in the markup
ojan: i prefer 2. it's hard to make sense of the serialized selection stuff in the renderdumps
othermaciej: 2. seems nice for reading though it's a bit harder to implement I think
othermaciej: since you can't just directly print document.innerHTML
othermaciej: (and then do separate selection dumping)
Comment 1 Maciej Stachowiak 2009-06-17 17:51:07 PDT
Darin suggested another potentially useful feature for dumpAsMarkup. He mentioned one thing we get out of the render tree dump is detecting when we inadvertantly make multiple adjacent text nodes. The editing code is not supposed to do that, but it is a common bug for it to accidentally do so. It would be very useful if markup dumping had an indicator for boundaries between multiple adjacent text nodes. That would automatically test for this bug in an even better way.
Comment 2 Tony Chang 2010-03-01 19:06:10 PST
This sounds similar to the output when calling dumpTree() from gdb on a node.  For position related nodes, it puts an asterisk on the left side of the line and write the offset at the end of the output.  It also shows cases where two text nodes are adjacent.  Maybe we can repurpose this output format?
Comment 3 Ojan Vafai 2010-03-01 19:23:20 PST
One thing that's lost in the dumpTree view is the value of node attributes. For the editing tests, we'll want to see things like font-family. 

I was picturing something that would just be a modified form of innerHTML (maybe innerHTML takes an optional boolean?). Modified to add info for textnodes (e.g. wrap text nodes in <webkit_test_text></webkit_test_text> tags) and selections (e.g. <webkit_test_selection></webkit_test_selection>).
Comment 4 Ojan Vafai 2010-03-24 18:00:19 PDT
Created attachment 51570 [details]
initial prototype

Not ready for review, but any feedback would be appreciated. Some questions I'd like answered:
1. Does this output format look good (see the test's expected results)?
2. Should I commit this layout test or just rely on tests actually using dumpAsMarkup to act as the testing?

Some things I'd like to do before putting it up for review:
1. Figure out how to include selection state
2. Make it work with other ports / TestShell
Comment 5 Ryosuke Niwa 2010-04-13 15:02:15 PDT
(In reply to comment #4)
> 1. Does this output format look good (see the test's expected results)?

The output looks nice but is it possible to format them as a tree with indentations?  e.g. instead of 

<html lang="en">
<body><text>\n </text> 
<text>\n </text><text>\n</text><text>\n</text></body> 


<html lang="en">
		<text>\n </text> 
		<text>\n </text>

This would improve the readability of the output at least for me.
Comment 6 Ojan Vafai 2010-04-26 18:27:57 PDT
Created attachment 54367 [details]
Comment 7 Ojan Vafai 2010-04-26 18:29:04 PDT
Darin and I talked about making this a JS file at the webkit conference. Here's my attempt. It's basically done except that I can't get the dumpAsText version to keep my indenting. Open to suggestions as to how to make it not strip whitespace.
Comment 8 Darin Adler 2010-04-27 12:48:43 PDT
Comment on attachment 54367 [details]

 +  \ No newline at end of file

Should have newlines at ends of files.
Comment 9 Ojan Vafai 2010-04-27 13:29:57 PDT
Committed r58331: <http://trac.webkit.org/changeset/58331>
Comment 10 WebKit Review Bot 2010-04-27 13:51:10 PDT
http://trac.webkit.org/changeset/58331 might have broken Qt Linux Release
The following changes are on the blame list:
Comment 11 Adam Barth 2010-04-27 13:53:37 PDT
Apparently this patch as a new test that fails on Qt.
Comment 12 Ojan Vafai 2010-04-27 13:56:52 PDT
(In reply to comment #11)
> Apparently this patch as a new test that fails on Qt.

Ugh. I forgot to reset the expected result after adding the newline. Fix coming as soon as my build finishes.
Comment 13 Ojan Vafai 2010-04-27 14:05:52 PDT
Created attachment 54452 [details]
Comment 14 Ojan Vafai 2010-04-27 14:06:46 PDT
Comment on attachment 54452 [details]

Forgot to update the test result after adding the newline. Quick commit to fix the now failing test.
Comment 15 Ojan Vafai 2010-04-27 14:06:49 PDT
Committed r58335: <http://trac.webkit.org/changeset/58335>