Bug 28471 - underline tests in /editing/style/ need not to be pixel tests but need to print markup
Summary: underline tests in /editing/style/ need not to be pixel tests but need to pri...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P1 Blocker
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks: 23892
  Show dependency treegraph
 
Reported: 2009-08-19 13:07 PDT by Ryosuke Niwa
Modified: 2009-08-20 22:40 PDT (History)
2 users (show)

See Also:


Attachments
patch 1 of 3 (72.25 KB, patch)
2009-08-19 14:24 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
1 of 3 (removed irrelevant changes) (71.15 KB, patch)
2009-08-19 14:25 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
1 of 3 (adding new runDumpAsTextEditingTest to editing.js) (64.50 KB, patch)
2009-08-19 15:49 PDT, Ryosuke Niwa
eric: review+
Details | Formatted Diff | Diff
2 of 3 (74.53 KB, patch)
2009-08-19 18:04 PDT, Ryosuke Niwa
eric: review+
Details | Formatted Diff | Diff
3 of 3 (82.82 KB, patch)
2009-08-19 18:19 PDT, Ryosuke Niwa
eric: review+
Details | Formatted Diff | Diff
additional patch for the record since Bugzilla data was lost (16.09 KB, patch)
2009-08-20 22:38 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 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.
Comment 1 Ryosuke Niwa 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
Comment 2 Ryosuke Niwa 2009-08-19 14:24:29 PDT
Created attachment 35144 [details]
patch 1 of 3
Comment 3 Ryosuke Niwa 2009-08-19 14:25:40 PDT
Created attachment 35145 [details]
1 of 3 (removed irrelevant changes)
Comment 4 Ryosuke Niwa 2009-08-19 15:49:27 PDT
Created attachment 35154 [details]
1 of 3 (adding new runDumpAsTextEditingTest to editing.js)
Comment 5 Eric Seidel (no email) 2009-08-19 15:52:02 PDT
Comment on attachment 35154 [details]
1 of 3 (adding new runDumpAsTextEditingTest to editing.js)

Looks OK.
Comment 6 Ryosuke Niwa 2009-08-19 17:17:30 PDT
The first patch committed as http://trac.webkit.org/changeset/47533.
Comment 7 Ryosuke Niwa 2009-08-19 18:04:18 PDT
Created attachment 35170 [details]
2 of 3
Comment 8 Ryosuke Niwa 2009-08-19 18:19:18 PDT
Created attachment 35171 [details]
3 of 3
Comment 9 Eric Seidel (no email) 2009-08-19 18:43:07 PDT
Comment on attachment 35170 [details]
2 of 3

Looks fine to me.
Comment 10 Eric Seidel (no email) 2009-08-19 18:43:39 PDT
Comment on attachment 35171 [details]
3 of 3

Looks fine to me.
Comment 11 Eric Seidel (no email) 2009-08-19 18:43:55 PDT
CCing justin so he sees these go by.
Comment 12 Ryosuke Niwa 2009-08-19 19:26:41 PDT
The second patch committed as http://trac.webkit.org/changeset/47542.
Comment 13 Ryosuke Niwa 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.
Comment 14 Ryosuke Niwa 2009-08-20 22:38:58 PDT
Created attachment 38359 [details]
additional patch for the record since Bugzilla data was lost
Comment 15 Ryosuke Niwa 2009-08-20 22:40:24 PDT
Fourth patch, which was r+ by Eric Seidel was committed as http://trac.webkit.org/changeset/47609.