Bug 10129 – REGRESSION: webViewDidChange: posted even when should* delegates reply NO

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Duncan Wilcox]

The following notification:

- (void)webViewDidChange:(NSNotification *)notification

is posted even when editing delegates reply NO to should* requests, like webView:shouldInsertText:replacingDOMRange: and webView:shouldDeleteDOMRange:.

I think the regression happened when some of the editing behaviour was moved from WebCore/bridge/mac to WebCore/page, but unfortunately I don't have an exact reference.

Unfortunately DRT doesn't currently have support for editing delegates returning NO, to veryify and check regressions. Adding this support is probably a good first step.

[Alexey Proskuryakov]

Regression->P1

[Duncan Wilcox]

Created attachment 9971 [details]
DumpRenderTree patch

Submitting this for feedback. This patch adds support for having the DumpRenderTree editing delegate respond NO to should* requests, for example with something like this:

Index: LayoutTests/editing/deleting/delete-by-word-001.html
===================================================================
--- LayoutTests/editing/deleting/delete-by-word-001.html        (revision 15828)
+++ LayoutTests/editing/deleting/delete-by-word-001.html        (working copy)
@@ -62,6 +62,8 @@
 </div>
 
 <script>
+if(window.layoutTestController)
+    window.layoutTestController.setAcceptsEditing(false);
 runEditingTest();
 </script>
 
This should perhaps initially only be enabled only on layout tests using the eventSender mechanism, as that path is more conformant to delegate requests (as opposed to the JS one).

I have a regression similar to the one in this bug where scrolling through text with arrow keys happens even if the delegate responds NO, should be relatively easy to build a test case.

[Duncan Wilcox]

Created attachment 9974 [details]
DumpRenderTree patch

Ooops, reset the acceptsEditing flag on each new test.

[Darin Adler]

Comment on attachment 9974 [details]
DumpRenderTree patch

Looks OK.

Formatting of this:

+        if(acceptsEditing) {

does not match our standard, and there is code here with braces around single-line blocks that also does not match it.

r=me

[Darin Adler]

Comment on attachment 9974 [details]
DumpRenderTree patch

I cut this down quite a bit before landing it, but it still should be sufficient to allow a test to be written.

Committed revision 15904.

Clearing the review flag on this so we can now use this bug report to fix the bug reported.

[Stephanie Lewis]

radar 4823005

[Mark Rowe (bdash)]

Radar shows this as resolved in r17981.