Created attachment 94533 [details] Work in Progress

Whoops, looks like I didn't send the right arguments to webkit-patch. This patch should have been titled "work in progress". I used the rebaseline-for-dumpas-conv script on this test, and it made the changes to the html file in this patch. But the output is text, not markup. Ryosuke, any comments on the correct way to convert a test that uses editing.js like this?

Your approach is correct and would have worked if this test had #root like other editing.js. See debugForDumpAsText in editing.js.

Thanks! I have this test converted, but before I post my patch, I wanted to add a description since I realized it doesn't have one. What's the correct way to put in a description for a test that calls runDumpAsTextEditingTest()? I see some tests have divs with classes "explanation", "scenario", "expected-results"--is that the right way to do it?

(In reply to comment #4) > Thanks! I have this test converted, but before I post my patch, I wanted to add a description since I realized it doesn't have one. What's the correct way to put in a description for a test that calls runDumpAsTextEditingTest()? I see some tests have divs with classes "explanation", "scenario", "expected-results"--is that the right way to do it? You can just add a p/div with explanation. I don't think those class names mean anything.

Created attachment 94651 [details] Patch

Comment on attachment 94651 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=94651&action=review Even though it's a minor change, I'd say r- since you're not a committer yet. > LayoutTests/ChangeLog:8 > + Converts delete-block-table.html to dump-as-markup by changing to use runDumpAsTextEditingTest. runDumpAsTextEditingTest don't use dump-as-markup.js so you should either say text test or dumpAsTest test.

Comment on attachment 94651 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=94651&action=review > LayoutTests/editing/deleting/delete-block-table.html:4 > <script src=../editing.js language="JavaScript" type="text/JavaScript" ></script> I would have stripped out language and type attributes.

Comment on attachment 94651 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=94651&action=review >> LayoutTests/editing/deleting/delete-block-table.html:4 >> <script src=../editing.js language="JavaScript" type="text/JavaScript" ></script> > > I would have stripped out language and type attributes. Also, I would have put all three script elements at the end of document.

Created attachment 94658 [details] Patch

Comment on attachment 94651 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=94651&action=review >> LayoutTests/ChangeLog:8 > > runDumpAsTextEditingTest don't use dump-as-markup.js so you should either say text test or dumpAsTest test. Done. >>> LayoutTests/editing/deleting/delete-block-table.html:4 >>> <script src=../editing.js language="JavaScript" type="text/JavaScript" ></script> >> >> I would have stripped out language and type attributes. > > Also, I would have put all three script elements at the end of document. Done. Since the two inline scripts are now next to each other, I combined them.

Comment on attachment 94658 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=94658&action=review The following comments for future references. > LayoutTests/editing/deleting/delete-block-table.html:27 > +<script src=../editing.js></script> I would prefer to wrap URL by (double) quotations but that's fine. > LayoutTests/editing/deleting/delete-block-table.html:37 > + for (i = 0; i < 3; i++) { > + deleteCommand(); > + } No curly brackets around a single statement.

Comment on attachment 94658 [details] Patch Rejecting attachment 94658 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-7', 'land-a..." exit_code: 1 Last 500 characters of output: autoinstalled/mechanize/_urllib2_fork.py", line 332, in _call_chain result = func(*args) File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_urllib2_fork.py", line 1170, in https_open return self.do_open(conn_factory, req) File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_urllib2_fork.py", line 1118, in do_open raise URLError(err) urllib2.URLError: <urlopen error [Errno 60] Operation timed out> Full output: http://queues.webkit.org/results/8732572

Comment on attachment 94658 [details] Patch Let's try again.

The commit-queue encountered the following flaky tests while processing attachment 94658 [details]: media/video-playbackrate.html bug 58629 (author: eric.carlson@apple.com) The commit-queue is continuing to process your patch.

Comment on attachment 94658 [details] Patch Clearing flags on attachment: 94658 Committed r87261: <http://trac.webkit.org/changeset/87261>

All reviewed patches have been landed. Closing bug.