Bug 31579 - Rewrite the layout test editing/inserting/6633727 to use dumpAsText
Summary: Rewrite the layout test editing/inserting/6633727 to use dumpAsText
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Minor
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-17 02:56 PST by Kinuko Yasuda
Modified: 2010-03-25 00:34 PDT (History)
4 users (show)

See Also:


Attachments
Rewrite the test editing/inserting/6633727 to use dumpAsText (29.54 KB, patch)
2009-11-17 03:07 PST, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Patch (29.85 KB, patch)
2010-03-04 20:44 PST, Kinuko Yasuda
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kinuko Yasuda 2009-11-17 02:56:22 PST
The layout test editing/inserting/6633727.html can be written using dumpAsText so that the results can be independent of minor platform differences.  (Currently it is failing on Chromium/Linux due to subtle font differences; rather than rebaselining we'll want to replace it with dumpAsText based testing.)
Comment 1 Kinuko Yasuda 2009-11-17 03:07:57 PST
Created attachment 43351 [details]
Rewrite the test editing/inserting/6633727 to use dumpAsText
Comment 2 Dmitry Titov 2009-11-20 13:38:34 PST
The test transformation by itself looks 'equivalent' to me, the render tree dump seem to correspond to html you verify. The dump also has caret information but it seems not needed because the test inserts '2' to verify where insertion point is.

I'm not sure I would convert this test to a script test though. It seems like 2 transformations at once - to 'dumpAsText' and to be a script text.

It'd be good if Adele took a look, both as author of the test and able to verify what that radar issue is exactly about.
Comment 3 Adam Barth 2009-11-30 12:25:30 PST
Attachment 43351 [details] passed the style-queue
Comment 4 David Levin 2009-12-17 14:24:56 PST
Comment on attachment 43351 [details]
Rewrite the test editing/inserting/6633727 to use dumpAsText

From reading the ChangeLog and bug description, it sounds like the bug is at least in part about where the insert position is at the end of this test. That aspect (where the cursor is) is no longer verified after this conversion. It is verified when there is an image because the cursor is visible in that image.
Comment 5 Kinuko Yasuda 2010-03-04 20:44:10 PST
Created attachment 50085 [details]
Patch
Comment 6 Kinuko Yasuda 2010-03-04 20:49:43 PST
(In reply to comment #4)
> (From update of attachment 43351 [details])
> From reading the ChangeLog and bug description, it sounds like the bug is at
> least in part about where the insert position is at the end of this test. That
> aspect (where the cursor is) is no longer verified after this conversion. It is
> verified when there is an image because the cursor is visible in that image.

Thanks for reviewing, I updated the patch to explicitly check the cursor position.
Comment 7 David Levin 2010-03-24 10:24:20 PDT
Comment on attachment 50085 [details]
Patch

It would be nice if the chromium pixel tests results were deleted with this submission (now that they are in the webkit repository).
Comment 8 WebKit Commit Bot 2010-03-25 00:34:14 PDT
Comment on attachment 50085 [details]
Patch

Clearing flags on attachment: 50085

Committed r56506: <http://trac.webkit.org/changeset/56506>
Comment 9 WebKit Commit Bot 2010-03-25 00:34:19 PDT
All reviewed patches have been landed.  Closing bug.