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>
Status: RESOLVED FIXED    
Severity: Blocker CC: commit-queue, ojan, tony
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 42436    
Attachments:
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-

Ryosuke Niwa
Reported 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.
Attachments
converts the tests (37.49 KB, patch)
2010-07-19 18:08 PDT, Ryosuke Niwa 		ojan: review-
ojan: commit-queue-
DetailsFormatted DiffDiff
removed create-list-with-hr.html and fixed per ojan's comments (27.09 KB, patch)
2010-07-19 19:03 PDT, Ryosuke Niwa 		ojan: review+
commit-queue: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2010-07-19 18:08:55 PDT
Created attachment 62019 [details] converts the tests
Ryosuke Niwa
Comment 2 2010-07-19 18:36:11 PDT
Will not convert create-list-with-hr.html since it demonstrates a rendering bug.
Ojan Vafai
Comment 3 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.
Ryosuke Niwa
Comment 4 2010-07-19 19:03:09 PDT
Created attachment 62027 [details] removed create-list-with-hr.html and fixed per ojan's comments
Ojan Vafai
Comment 5 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!
WebKit Commit Bot
Comment 6 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: ommand/create-list-from-range-selection-expected.txt' 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
Ryosuke Niwa
Comment 7 2010-07-21 21:03:11 PDT
Landed as http://trac.webkit.org/changeset/63873.
Note You need to log in before you can comment on or make changes to this bug.