Some test cases in /LayoutTests/editing/execCommand are using a pixel test when a simple dumpAsText test would suffice the purpose. This slows down the LayoutTexts considerably.
Comment on attachment 31184[details]
Fixes the bug. Most of test cases now use a dumpAsText test.
I can't review 20 tests at once. Please post these in smaller patches. Ideally 20k or less.
Comment on attachment 31436[details]
converts 5432254-2.html 5700414-1.html 5700414-2.html
Typo in:
LayoutTests/editing/execCommand/5700414-2.html
4 <div id="div">There should be be a single empty paragraph above.</div>
Otherwise looks fine.
Comment on attachment 31437[details]
converts find-after-replace.html findString-2.html insert-list-and-stitch.html
Tab:
if (window.layoutTestController) {
3 layoutTestController.dumpEditingCallbacks();
4 layoutTestController.dumpAsText();
5 }
Is there no button?
5 <p>The three items below should be stitched together into one ordered list when you click the button.</div>
Comment on attachment 31438[details]
converts insertHTML.html switch-list-type.html
Looks OK. Sad we can't pretty-print this:
36 <ol style="border: 1px solid red;"> <li>This should be in an ordered list.</li> </ol><span id="item1"><ul><li><span id="item1">This should be in an unordered list.</span> </li></ul></span><ol style="border: 1px solid red;"><br> This should be in an ordered list. <li>This should be in an ordered list.</li> </ol><ul><li>This should be in an unordered list.</li></ul><ol style="border: 1px solid red;"><li>This should be in an ordered list.</li> </ol><ul><li>This should be in an unordered list.</li></ul><ol style="border: 1px solid red;"> <li>This should be in an ordered list.</li> </ol> This should not be in a list.<br><ol><li>This should be in an ordered list.</li></ol>
Comment on attachment 31439[details]
converts paste-1.html paste-2.html
Tabs:
0 if (window.layoutTestController)
20 window.layoutTestController.notifyDone();
31 window.layoutTestController.notifyDone();
Missing newline:
2539 </script>
2640 \ No newline at end of file
Otherwise looks fine. The tab will block checkin (the pre-commit hook will fail).
(In reply to comment #28)
> Sad we can't pretty-print this:
> 36 <ol style="border: 1px solid red;"> <li>This should be in an ordered
> list.</li> </ol><span id="item1"><ul><li><span id="item1">This should be in an
> unordered list.</span> </li></ul></span><ol style="border: 1px solid
> red;"><br> This should be in an ordered list. <li>This should be in an ordered
> list.</li> </ol><ul><li>This should be in an unordered list.</li></ul><ol
> style="border: 1px solid red;"><li>This should be in an ordered list.</li>
> </ol><ul><li>This should be in an unordered list.</li></ul><ol style="border:
> 1px solid red;"> <li>This should be in an ordered list.</li> </ol> This should
> not be in a list.<br><ol><li>This should be in an ordered list.</li></ol>
But we can! There's no reason we just have to log innerHTML. We could use a couple regular expressions to make it easier to read. I would if I was writing a test with this as the result.
Comment on attachment 31440[details]
converts remove-list-1.html remove-list-items.html
Tab:
3 layoutTestController.dumpEditingCallbacks();
4 layoutTestController.dumpAsText();
Why?
<div id="div" contenteditable="true"><ol><li></li><li>There should be a empty paragraph above this one.</li></ol></div>
8 <div id="div" contenteditable="true"><ol><li></li><li>There should be a single BR above this line and no OL or LI.</li></ol></div>
(In reply to comment #31)
> Why?
> <div id="div" contenteditable="true"><ol><li></li><li>There should be a empty
> paragraph above this one.</li></ol></div>
> 8 <div id="div" contenteditable="true"><ol><li></li><li>There should be a
> single BR above this line and no OL or LI.</li></ol></div>
>
Because if we simply deleted LI tags, then there will be no content above the sentence; i.e. if acts as if we set display:none; for the first LI. But since we're just untoggling ordered list, there should still be a space allocated for it. As a place holder, we insert a BR.
(In reply to comment #27)
> Is there no button?
> 5 <p>The three items below should be stitched together into one ordered list
> when you click the button.</div>
As far as I checked, there was no button.
(In reply to comment #46)
> Which ones of these have already been landed? It would be best if they were
> already obsoleted so that the rest of these could be more easily landed.
>
obsolated 4 patches that have already been landed.
Comment on attachment 31458[details]
converts 5432254-2.html 5700414-1.html 5700414-2.html
Committing to http://svn.webkit.org/repository/webkit/trunk ...
D LayoutTests/platform/mac/editing/execCommand/5432254-2-expected.checksum
D LayoutTests/platform/mac/editing/execCommand/5432254-2-expected.png
D LayoutTests/platform/mac/editing/execCommand/5432254-2-expected.txt
D LayoutTests/platform/mac/editing/execCommand/5700414-1-expected.checksum
D LayoutTests/platform/mac/editing/execCommand/5700414-1-expected.png
D LayoutTests/platform/mac/editing/execCommand/5700414-1-expected.txt
D LayoutTests/platform/mac/editing/execCommand/5700414-2-expected.checksum
D LayoutTests/platform/mac/editing/execCommand/5700414-2-expected.png
D LayoutTests/platform/mac/editing/execCommand/5700414-2-expected.txt
D LayoutTests/platform/qt/editing/execCommand/5432254-2-expected.txt
M LayoutTests/ChangeLog
A LayoutTests/editing/execCommand/5432254-2-expected.txt
M LayoutTests/editing/execCommand/5432254-2.html
A LayoutTests/editing/execCommand/5700414-1-expected.txt
M LayoutTests/editing/execCommand/5700414-1.html
A LayoutTests/editing/execCommand/5700414-2-expected.txt
M LayoutTests/editing/execCommand/5700414-2.html
Committed r45224
Comment on attachment 31461[details]
converts remove-list-1.html remove-list-items.html
Committing to http://svn.webkit.org/repository/webkit/trunk ...
R LayoutTests/platform/mac/editing/execCommand/remove-list-1-expected.txt => LayoutTests/editing/execCommand/remove-list-1-expected.txt
D LayoutTests/platform/mac/editing/execCommand/remove-list-1-expected.checksum
D LayoutTests/platform/mac/editing/execCommand/remove-list-1-expected.png
D LayoutTests/platform/mac/editing/execCommand/remove-list-items-expected.checksum
D LayoutTests/platform/mac/editing/execCommand/remove-list-items-expected.png
D LayoutTests/platform/mac/editing/execCommand/remove-list-items-expected.txt
D LayoutTests/platform/qt/editing/execCommand/remove-list-1-expected.txt
D LayoutTests/platform/qt/editing/execCommand/remove-list-items-expected.txt
M LayoutTests/ChangeLog
M LayoutTests/editing/execCommand/remove-list-1.html
A LayoutTests/editing/execCommand/remove-list-items-expected.txt
M LayoutTests/editing/execCommand/remove-list-items.html
Committed r45239http://trac.webkit.org/changeset/45239
Comment on attachment 31464[details]
converts find-after-replace.html findString-2.html insert-list-and-stitch.html
After applying the patch:
editing/execCommand/find-after-replace.html -> failed
Given that we seem to want to take a different approach for these. I think we can just close this bug now. The last two patches which failed, there seems little point in updating them. Feel-free to re-open if you disagree.
(In reply to comment #52)
> Given that we seem to want to take a different approach for these. I think we
> can just close this bug now. The last two patches which failed, there seems
> little point in updating them. Feel-free to re-open if you disagree.
>
I think those are failing because my patch for 14062. I see weird nbsp's in the expected results for paste-1/2 & find-after-replace. It's interesting that these bug didn't show up in layout tests because a regular space and non-breaking space are rendered same way in many cases.
2009-06-11 17:42 PDT, Ryosuke Niwa
2009-06-16 15:37 PDT, Ryosuke Niwa
2009-06-16 15:38 PDT, Ryosuke Niwa
2009-06-16 15:38 PDT, Ryosuke Niwa
2009-06-16 15:39 PDT, Ryosuke Niwa
2009-06-16 15:39 PDT, Ryosuke Niwa
2009-06-16 15:40 PDT, Ryosuke Niwa
2009-06-16 15:41 PDT, Ryosuke Niwa
2009-06-17 14:24 PDT, Ryosuke Niwa
2009-06-17 14:29 PDT, Ryosuke Niwa
2009-06-17 14:30 PDT, Ryosuke Niwa
2009-06-17 14:32 PDT, Ryosuke Niwa
2009-06-17 14:33 PDT, Ryosuke Niwa
2009-06-17 14:36 PDT, Ryosuke Niwa
2009-06-17 14:37 PDT, Ryosuke Niwa
2009-06-17 14:39 PDT, Ryosuke Niwa
2009-06-17 14:41 PDT, Ryosuke Niwa
2009-06-17 16:18 PDT, Ryosuke Niwa
2009-06-17 16:22 PDT, Ryosuke Niwa
2009-06-17 16:25 PDT, Ryosuke Niwa
2009-06-17 16:31 PDT, Ryosuke Niwa
2009-06-17 16:33 PDT, Ryosuke Niwa
2009-06-17 16:35 PDT, Ryosuke Niwa