Bug 43015

Summary: dump-as-markup should have better output
Product: WebKit Reporter: Ojan Vafai <ojan>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, darin, eric, james, mjs, rniwa, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch abarth: review+

Description Ojan Vafai 2010-07-26 16:01:12 PDT
Feedback from the webkit-dev thread.

1. Selection markers should be # prefixed
2. nodenames should be lowercase (use localName instead of nodeName)
3. Text nodes should be delimited by quotes, not <#text>
4. Should indent.
Comment 1 Ojan Vafai 2010-07-26 16:28:08 PDT
Also, there are some things that LayoutTests/html5lib/resources/dom2string.js does that dump-as-markup should do as well:
-prints doctype,
-deals with misnested DOMs
-comments nodes
-namespaced nodes/attributes (e.g. svg/math)
Comment 2 Ojan Vafai 2010-07-27 17:33:50 PDT
Created attachment 62778 [details]
Patch
Comment 3 Adam Barth 2010-07-27 19:29:17 PDT
Comment on attachment 62778 [details]
Patch

Very cool.  Thanks for doing this.  Unfortunately, we can't change the format of the html5lib test expectations.  These data files are just a copy of the "official" ones at <http://code.google.com/p/html5lib/source/browse/#hg/testdata/tree-construction>.  The data format in them is chosen to work with a number of different HTML5 parsers in a bunch of different languages (including many that don't execute JavaScript).

Can we keep the new dump-as-markup but leave the HTML5lib test suite using the "old and busted" dom2string?  Alternatively, we could dump-as-markup produce identical dumps for the HTML5lib test suite, but that might constrain our ability to make dom-as-markup more awesome in the future.
Comment 4 Ryosuke Niwa 2010-07-27 19:41:14 PDT
(In reply to comment #3)
> Can we keep the new dump-as-markup but leave the HTML5lib test suite using the "old and busted" dom2string?  Alternatively, we could dump-as-markup produce identical dumps for the HTML5lib test suite, but that might constrain our ability to make dom-as-markup more awesome in the future.

We could pass some flag. e.g. In the .js file loaded for HTML5lib tests, we set Markup.html5lib = true.  I don't know... maybe it's not worth combining the code if we have two modes.
Comment 5 Adam Barth 2010-07-27 19:57:13 PDT
The flag sounds like a good idea.  I don't quite understand where the differences are coming from, so I'm not sure if they're bugs.  Maybe they dump document fragments differently?
Comment 6 Ojan Vafai 2010-07-28 15:37:44 PDT
Created attachment 62888 [details]
Patch
Comment 7 Ojan Vafai 2010-07-28 15:44:51 PDT
I've made the output match. The only difference before was whether we print the DOM root node passed in.

Having as few testing frameworks as possible is a good thing for keeping our tests easy to work with. If we run into something we want to add to dump-as-markup that doesn't work for the html5lib tests, then we can add a flag then. For now, we can have the same output.
Comment 8 Adam Barth 2010-07-29 08:10:53 PDT
Comment on attachment 62888 [details]
Patch

Fantastic.  Thanks Ojan.

LayoutTests/html5lib/runner.html:94
 +          layoutTestController.notifyDone();
I'm curious why you added this.  DRT waits until after the load event before finishing.
Comment 9 Ojan Vafai 2010-07-29 11:54:07 PDT
(In reply to comment #8)
>  +          layoutTestController.notifyDone();
> I'm curious why you added this.  DRT waits until after the load event before finishing.

This is just the dump-as-markup API being dumb. Instead, I added Markup.noAutoDump, replaced the call to Markup.waitUntilDone, and got rid of the notifyDone call.
Comment 10 Ojan Vafai 2010-07-29 13:17:05 PDT
Committed r64303: <http://trac.webkit.org/changeset/64303>
Comment 11 WebKit Review Bot 2010-07-29 13:34:24 PDT
http://trac.webkit.org/changeset/64303 might have broken Qt Linux Release