Bug 4011 - Editing delegate selection methods not called when using mouse
Summary: Editing delegate selection methods not called when using mouse
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Major
Assignee: Dave Hyatt
URL:
Keywords:
Depends on: 3734
Blocks: 4375
  Show dependency treegraph
 
Reported: 2005-07-15 06:18 PDT by Duncan Wilcox
Modified: 2005-08-14 19:59 PDT (History)
1 user (show)

See Also:


Attachments
test case (1.69 KB, application/octet-stream)
2005-07-15 06:23 PDT, Duncan Wilcox
no flags Details
patch with changelog entries (13.50 KB, patch)
2005-07-15 12:49 PDT, Duncan Wilcox
darin: review-
Details | Formatted Diff | Diff
patch with changelog entries (15.54 KB, patch)
2005-08-01 17:31 PDT, Duncan Wilcox
darin: review+
Details | Formatted Diff | Diff
layout test (crashes webcore) (800 bytes, text/plain)
2005-08-06 09:00 PDT, Duncan Wilcox
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Duncan Wilcox 2005-07-15 06:18:22 PDT
webView:shouldChangeSelectedDOMRange:toDOMRange:affinity:stillSelecting: should be called before 
webViewDidChangeSelection: when using the mouse on an editable area:

- moving the cursor with arrow keys, with or without extending the selection with shift, will call both the 
SHOULD and the DID method
- clicking or doubleclicking will call the DID method, but not the SHOULD method
Comment 1 Duncan Wilcox 2005-07-15 06:23:10 PDT
Created attachment 2973 [details]
test case

Create a cocoa app in xcode, replace main.m with the attachment and add the
WeKit.framework to the project.
Comment 2 Duncan Wilcox 2005-07-15 12:49:44 PDT
Created attachment 2975 [details]
patch with changelog entries

This is the first patch I develop that drills through all the glue layers,
please advise.
Comment 3 Dan Wood 2005-07-15 16:50:20 PDT
It looks like this bug may be a partial duplicate of 3967.

Comment 4 Darin Adler 2005-07-18 08:21:02 PDT
Comment on attachment 2975 [details]
patch with changelog entries

Formatting problem -- if statements say "if(" and should say "if (" instead.

Ideally we should also have a layout test. We added support to the layout test
engine for reporting editing delegate output, and you can create mouse events
using the DOM events support -- with these two things together you should be
able to make a layout test that would work.

Otherwise, looks good.
Comment 5 Duncan Wilcox 2005-08-01 17:31:29 PDT
Created attachment 3201 [details]
patch with changelog entries

Updated for current HEAD, seems to work fine.
Comment 6 Darin Adler 2005-08-04 11:25:08 PDT
Comment on attachment 3201 [details]
patch with changelog entries

Looks good.

Marking review- because there's still no layout test. If there was a test I'd
have done review+.
Comment 7 Dan Wood 2005-08-05 11:34:02 PDT
Darin, will the test cases from 3967 suffice?  As I mentioned above, this bug is pretty much a duplicate of 
that, so the test cases I generated for that should work.  Do I need to upload them and explicitly attach to 
this bug?
Comment 8 Duncan Wilcox 2005-08-06 09:00:51 PDT
Created attachment 3240 [details]
layout test (crashes webcore)

This is an attempt at building a layout test that should use the new editing
delegate support in DumpRenderTree. Unfortunately I don't know much about DOM2
events, so I don't know if my JS code is correct. I can only get WebCore to
crash.

I will file the bug for the crash, I would need someone to look at the JS in
the layout test to know if it is correct, and this bug should depend on the
layout test crash bug.
Comment 9 Darin Adler 2005-08-07 09:49:15 PDT
A test program is not a substitute for a layout test. We don't have any way to run a compiled test program 
in an automated way. So the tests attached to bug 3967 don't help with the lack of layout test.
Comment 10 Darin Adler 2005-08-07 09:55:35 PDT
Comment on attachment 3201 [details]
patch with changelog entries

I guess we can't easily make a layout test for this, so r=me.
Comment 11 Dan Wood 2005-08-10 12:20:59 PDT
I've tried out this patch against the test application that I made for bug 3967, and I can confirm that the 
callback I is getting called in my test.

What do we need to do to get this checked in? :-)