Bug 42608

Summary: dumpAsMarkup test conversion: create-list-from-range-selection.html and insert-list-empty-div.html
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: HTML EditingAssignee: Ryosuke Niwa <rniwa>
Severity: Blocker CC: commit-queue, ojan, tony
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 42436    
Description Flags
converts the tests
ojan: review-, ojan: commit-queue-
removed create-list-with-hr.html and fixed per ojan's comments ojan: review+, commit-queue: commit-queue-

Description Ryosuke Niwa 2010-07-19 17:42:33 PDT
create-list-from-range-selection.html, create-list-with-hr.html, and insert-list-empty-div.html currently uses render test but they should all be using dumpAsMarkup test since what we need to know is the final DOM tree and the selection.
Comment 1 Ryosuke Niwa 2010-07-19 18:08:55 PDT
Created attachment 62019 [details]
converts the tests
Comment 2 Ryosuke Niwa 2010-07-19 18:36:11 PDT
Will not convert create-list-with-hr.html since it demonstrates a rendering bug.
Comment 3 Ojan Vafai 2010-07-19 18:37:48 PDT
Comment on attachment 62019 [details]
converts the tests

> Index: LayoutTests/resources/dump-as-markup.js
> ===================================================================
> --- LayoutTests/resources/dump-as-markup.js	(revision 63331)
> +++ LayoutTests/resources/dump-as-markup.js	(working copy)
> @@ -17,7 +17,7 @@ Markup.dump = function(opt_node)

> +Markup.setNode = function(node)

Can we call this setNodeToDump so it's more clear what it does? 

> +{
> +  if (typeof node == "string")
> +    node = document.getElementById(node);
> +  if (node instanceof Node)

I don't think you need this if-check. If node is not a Node here, we should error out.

> +    Markup.node = node

This should be private, i.e., Markup._node.
Comment 4 Ryosuke Niwa 2010-07-19 19:03:09 PDT
Created attachment 62027 [details]
removed create-list-with-hr.html and fixed per ojan's comments
Comment 5 Ojan Vafai 2010-07-20 16:05:34 PDT
Comment on attachment 62027 [details]
removed create-list-with-hr.html and fixed per ojan's comments

Wow. It's so much easier to read these expected results!
Comment 6 WebKit Commit Bot 2010-07-20 17:50:19 PDT
Comment on attachment 62027 [details]
removed create-list-with-hr.html and fixed per ojan's comments

Rejecting patch 62027 from commit-queue.

Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Ojan Vafai', u'--force']" exit_code: 1
Last 500 characters of output:
patching file LayoutTests/platform/qt/editing/execCommand/insert-list-empty-div-expected.txt
rm 'LayoutTests/platform/qt/editing/execCommand/insert-list-empty-div-expected.txt'
patching file LayoutTests/resources/dump-as-markup.js
Hunk #2 FAILED at 31.
Hunk #3 succeeded at 65 with fuzz 1 (offset 10 lines).
Hunk #4 succeeded at 237 (offset 12 lines).
1 out of 4 hunks FAILED -- saving rejects to file LayoutTests/resources/dump-as-markup.js.rej

Full output: http://queues.webkit.org/results/3395665
Comment 7 Ryosuke Niwa 2010-07-21 21:03:11 PDT
Landed as http://trac.webkit.org/changeset/63873.