Bug 10129 - REGRESSION: webViewDidChange: posted even when should* delegates reply NO
Summary: REGRESSION: webViewDidChange: posted even when should* delegates reply NO
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Nobody
URL:
Keywords: InRadar, Regression
Depends on:
Blocks:
 
Reported: 2006-07-27 06:52 PDT by Duncan Wilcox
Modified: 2007-02-06 22:56 PST (History)
1 user (show)

See Also:


Attachments
DumpRenderTree patch (9.14 KB, patch)
2006-08-10 15:40 PDT, Duncan Wilcox
no flags Details | Formatted Diff | Diff
DumpRenderTree patch (9.62 KB, patch)
2006-08-10 21:51 PDT, Duncan Wilcox
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Duncan Wilcox 2006-07-27 06:52:19 PDT
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.
Comment 1 Alexey Proskuryakov 2006-07-27 09:09:01 PDT
Regression->P1
Comment 2 Duncan Wilcox 2006-08-10 15:40:34 PDT
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.
Comment 3 Duncan Wilcox 2006-08-10 21:51:25 PDT
Created attachment 9974 [details]
DumpRenderTree patch

Ooops, reset the acceptsEditing flag on each new test.
Comment 4 Darin Adler 2006-08-12 18:03:26 PDT
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
Comment 5 Darin Adler 2006-08-15 20:47:43 PDT
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.
Comment 6 Stephanie Lewis 2006-11-06 19:06:27 PST
radar 4823005
Comment 7 Mark Rowe (bdash) 2007-02-06 22:56:18 PST
Radar shows this as resolved in r17981.