Bug 20655 - Layout tests need a way to disable smart insert or delete
Summary: Layout tests need a way to disable smart insert or delete
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P4 Minor
Assignee: Darin Adler
URL:
Keywords:
Depends on:
Blocks: 15643
  Show dependency treegraph
 
Reported: 2008-09-04 18:55 PDT by Glenn Wilson
Modified: 2008-10-16 08:54 PDT (History)
1 user (show)

See Also:


Attachments
Possible fix to bug 20655 (8.04 KB, patch)
2008-09-07 17:37 PDT, Glenn Wilson
sam: review-
Details | Formatted Diff | Diff
Possible fix to bug 20655 (8.88 KB, patch)
2008-09-09 18:18 PDT, Glenn Wilson
no flags Details | Formatted Diff | Diff
Possible fix w/ mac support (8.88 KB, patch)
2008-09-09 18:43 PDT, Glenn Wilson
no flags Details | Formatted Diff | Diff
Possible fix to bug 20655 (9.86 KB, patch)
2008-09-11 19:07 PDT, Glenn Wilson
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Glenn Wilson 2008-09-04 18:55:07 PDT
Currently, smartInsertOrDelete is always on by default (it is set to true when the WebView is created), and there is no way for a layout test to set it to false.  

Ideally, layout tests would be able to toggle this flag to test features that are smartInsertOrDelete sensitive.
Comment 1 Glenn Wilson 2008-09-07 17:37:59 PDT
Created attachment 23232 [details]
Possible fix to bug 20655

Here is a possible fix to this issue.

This patch adds a new method to LayoutTestController, "setSmartInsertDeleteEnabled(bool)", which allows the test to set or unset this behavior in the webView.  Also added a test that illustrates that when smart editing is disabled, smart deleting behavior changes -- in fact, the test is just one of the smart delete tests modified to have this setting disabled.

As far as I can tell, disabling smart editing in one test does not affect others.  Tests before and after this test that require smart editing still pass on my system.

However, I am only able to test this on a Windows system, so many of the Mac-only layout tests don't run for me regardless.  Is there a way for me to try this patch on a publicly available Mac builder, or is there a way that a reviewer help me with this?
Comment 2 Sam Weinig 2008-09-09 13:26:37 PDT
Comment on attachment 23232 [details]
Possible fix to bug 20655

This will break the mac build as you have not included a version of LayoutTestController::setSmartInsertDeleteEnabled for the mac.
Comment 3 Glenn Wilson 2008-09-09 18:18:03 PDT
Created attachment 23308 [details]
Possible fix to bug 20655

Here is an improved patch that also modifies the mac/LayoutTestController.

Should this also modify the Gtk / Qt versions of LayoutTestController as well?
Comment 4 Glenn Wilson 2008-09-09 18:33:41 PDT
Comment on attachment 23308 [details]
Possible fix to bug 20655

oops, found a bug in this...
Comment 5 Glenn Wilson 2008-09-09 18:43:07 PDT
Created attachment 23310 [details]
Possible fix w/ mac support

This should also modify LayoutTestControllerMac.mm...sorry about that.
Comment 6 Mark Rowe (bdash) 2008-09-09 19:19:02 PDT
Do we need to reset the state of smart insert/delete between tests?
Comment 7 Glenn Wilson 2008-09-10 16:08:00 PDT
Comment on attachment 23310 [details]
Possible fix w/ mac support

From just a quick test, yes, it does look like it needs to be reset.  Working on an improved patch now...
Comment 8 Glenn Wilson 2008-09-11 19:07:07 PDT
Created attachment 23362 [details]
Possible fix to bug 20655

Here is the fix with a modified DumpRenderTree that resets the smartInsertDeleteEnabled flag to true after each test.  I tested this by adding a test that required smart delete that would run last, after the tests that disabled smartInsertDelete.   I didn't include this "teardown" test in the patch, though, since it is merely one of the existing smart deleting tests.
Comment 9 Adam Barth 2008-10-14 01:22:00 PDT
Will land.
Comment 10 Adam Barth 2008-10-14 03:00:55 PDT
My Mac Mini died in the middle of testing this patch.  I'm going to take it into the store tomorrow.
Comment 11 Adam Barth 2008-10-14 12:27:15 PDT
It's going to be a week before I get my Mac Mini back.  Unassigning.
Comment 12 Darin Adler 2008-10-15 20:00:32 PDT
I'll land this.
Comment 13 Darin Adler 2008-10-16 08:54:49 PDT
http://trac.webkit.org/changeset/37633