RESOLVED FIXED 28471
underline tests in /editing/style/ need not to be pixel tests but need to print markup 
https://bugs.webkit.org/show_bug.cgi?id=28471
Summary underline tests in /editing/style/ need not to be pixel tests but need to pri...
Ryosuke Niwa
Reported 2009-08-19 13:07:29 PDT
Because we rendering of underline is already tested, underline tests in /editing/style need not to be pixel tests. However, we care about whether texts are really underlined and what kind of markup is used to underline them. The current pixel test do not only reveal those information, and hence needs to be converted to dumpAsText tests with markup printed at the end.
Attachments
patch 1 of 3 (72.25 KB, patch)
2009-08-19 14:24 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
1 of 3 (removed irrelevant changes) (71.15 KB, patch)
2009-08-19 14:25 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
1 of 3 (adding new runDumpAsTextEditingTest to editing.js) (64.50 KB, patch)
2009-08-19 15:49 PDT, Ryosuke Niwa 		eric: review+
DetailsFormatted DiffDiff
2 of 3 (74.53 KB, patch)
2009-08-19 18:04 PDT, Ryosuke Niwa 		eric: review+
DetailsFormatted DiffDiff
3 of 3 (82.82 KB, patch)
2009-08-19 18:19 PDT, Ryosuke Niwa 		eric: review+
DetailsFormatted DiffDiff
additional patch for the record since Bugzilla data was lost (16.09 KB, patch)
2009-08-20 22:38 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2009-08-19 13:19:04 PDT
Minimum set of tests that do require conversion. editing/style/remove-underline-across-paragraph-in-bold.html editing/style/remove-underline-across-paragraph.html editing/style/remove-underline-after-paragraph-in-bold.html editing/style/remove-underline-after-paragraph.html editing/style/remove-underline.html editing/style/underline.html
Ryosuke Niwa
Comment 2 2009-08-19 14:24:29 PDT
Created attachment 35144 [details] patch 1 of 3
Ryosuke Niwa
Comment 3 2009-08-19 14:25:40 PDT
Created attachment 35145 [details] 1 of 3 (removed irrelevant changes)
Ryosuke Niwa
Comment 4 2009-08-19 15:49:27 PDT
Created attachment 35154 [details] 1 of 3 (adding new runDumpAsTextEditingTest to editing.js)
Eric Seidel (no email)
Comment 5 2009-08-19 15:52:02 PDT
Comment on attachment 35154 [details] 1 of 3 (adding new runDumpAsTextEditingTest to editing.js) Looks OK.
Ryosuke Niwa
Comment 6 2009-08-19 17:17:30 PDT
The first patch committed as http://trac.webkit.org/changeset/47533.
Ryosuke Niwa
Comment 7 2009-08-19 18:04:18 PDT
Created attachment 35170 [details] 2 of 3
Ryosuke Niwa
Comment 8 2009-08-19 18:19:18 PDT
Created attachment 35171 [details] 3 of 3
Eric Seidel (no email)
Comment 9 2009-08-19 18:43:07 PDT
Comment on attachment 35170 [details] 2 of 3 Looks fine to me.
Eric Seidel (no email)
Comment 10 2009-08-19 18:43:39 PDT
Comment on attachment 35171 [details] 3 of 3 Looks fine to me.
Eric Seidel (no email)
Comment 11 2009-08-19 18:43:55 PDT
CCing justin so he sees these go by.
Ryosuke Niwa
Comment 12 2009-08-19 19:26:41 PDT
The second patch committed as http://trac.webkit.org/changeset/47542.
Ryosuke Niwa
Comment 13 2009-08-19 19:45:11 PDT
The third patch committed as http://trac.webkit.org/changeset/47543. The second patch wasn't committed properly and fixed in http://trac.webkit.org/changeset/47544.
Ryosuke Niwa
Comment 14 2009-08-20 22:38:58 PDT
Created attachment 38359 [details] additional patch for the record since Bugzilla data was lost
Ryosuke Niwa
Comment 15 2009-08-20 22:40:24 PDT
Fourth patch, which was r+ by Eric Seidel was committed as http://trac.webkit.org/changeset/47609.
Note You need to log in before you can comment on or make changes to this bug.