Bug 26501

Summary:

layoutTestController should have dumpAsMarkup

Product:

WebKit

Reporter:

Ojan Vafai <ojan>

Component:

Tools / Tests

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, ap, aroben, eric, hamaji, jmalonzo, jparent, levin, mrowe, rniwa, tonikitoo, tony, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Mac

OS:

OS X 10.5

Attachments:

Description	Flags
initial prototype	none
Patch-14001	none
Patch	none

Ojan Vafai

Reported 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 <snip>...</snip> 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)

Attachments
initial prototype (21.88 KB, patch) 2010-03-24 18:00 PDT, Ojan Vafai	no flags	Details Formatted Diff Diff
Patch-14001 (10.17 KB, patch) 2010-04-26 18:27 PDT, Ojan Vafai	no flags	Details Formatted Diff Diff
Patch (1.19 KB, patch) 2010-04-27 14:05 PDT, Ojan Vafai	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Maciej Stachowiak

Comment 1 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.

Tony Chang

Comment 2 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?

Ojan Vafai

Comment 3 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>).

Ojan Vafai

Comment 4 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

Ryosuke Niwa

Comment 5 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> <p></p> <text>\n </text><text>\n</text><text>\n</text></body> </html> get <html lang="en"> <body> <text>\n </text> <p></p> <text>\n </text> <text>\n</text> <text>\n</text> </body> </html> This would improve the readability of the output at least for me.

Ojan Vafai

Comment 6 2010-04-26 18:27:57 PDT

Created attachment 54367 [details] Patch-14001

Ojan Vafai

Comment 7 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.

Darin Adler

Comment 8 2010-04-27 12:48:43 PDT

Comment on attachment 54367 [details] Patch-14001 LayoutTests/editing/selection/dump-as-markup.html:16 + \ No newline at end of file Should have newlines at ends of files.

Ojan Vafai

Comment 9 2010-04-27 13:29:57 PDT

Committed r58331: <http://trac.webkit.org/changeset/58331>

WebKit Review Bot

Comment 10 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: http://trac.webkit.org/changeset/58329 http://trac.webkit.org/changeset/58330 http://trac.webkit.org/changeset/58331

Adam Barth

Comment 11 2010-04-27 13:53:37 PDT

Apparently this patch as a new test that fails on Qt.

Ojan Vafai

Comment 12 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.

Ojan Vafai

Comment 13 2010-04-27 14:05:52 PDT

Created attachment 54452 [details] Patch

Ojan Vafai

Comment 14 2010-04-27 14:06:46 PDT

Comment on attachment 54452 [details] Patch Forgot to update the test result after adding the newline. Quick commit to fix the now failing test.

Ojan Vafai

Comment 15 2010-04-27 14:06:49 PDT

Committed r58335: <http://trac.webkit.org/changeset/58335>

Note You need to log in before you can comment on or make changes to this bug.