Bug 11323 - Link dragging behaviour does not obey WebKitEditableLinkBehavior WebPref
Summary: Link dragging behaviour does not obey WebKitEditableLinkBehavior WebPref
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Graham Dennis
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-10-17 02:24 PDT by Graham Dennis
Modified: 2006-11-06 14:53 PST (History)
2 users (show)

See Also:


Attachments
patch (14.83 KB, patch)
2006-10-17 20:33 PDT, Graham Dennis
timothy: review-
Details | Formatted Diff | Diff
patch 2 (15.63 KB, patch)
2006-11-05 23:15 PST, Graham Dennis
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Graham Dennis 2006-10-17 02:24:27 PDT
Currently, no editable links can be dragged (as links) if the default UI delegate's method for
- (unsigned)webView:(WebView *)webView dragSourceActionMaskForPoint:(NSPoint)point;
is used. For example, if WebKitEditableLinkBehavior (see bug 10338) is set to WebKitEditableLinkAlwaysLive (the default), although the link is live, it cannot be dragged.

The way that I would fix this would be to modify -[WebHTMLView _mayStartDragAtEventLocation:] to make the decision as to whether or not a drag is possible based on the WebPref and the action mask. However, for this to have any impact at all, either the default UI delegate needs to be changed to return WebDragSourceActionAny (instead of checking to see if the clicked element is editable), or the UI delegate needs to be overridden in the application. From a compatibility point of view, it would make sense to me not to change the default UI delegate (as the behaviour of editable links in, for example, Mail.app would change), however this means that for an app to change the editable link behaviour they would need to both set the WebPref and override the UI delegate. I'm not saying this is an onerous task, I just want to know whether or not the default UI delegate should be changed.

Changing the default UI delegate would have an impact on Mail.app as current behaviour in (Tiger) Mail is for editable links to be active, but dragging them just selects the text. Making this change would mean that dragging a link in the default configuration would drag the link and not select the text.

So, is this a bug and should the default UI delegate change?

This bug follows on from Bug 10338.
Comment 1 Graham Dennis 2006-10-17 20:33:12 PDT
Created attachment 11128 [details]
patch

This patch modifies the UI delegate so that it can be tested without writing a wrapper application.
Other changes include clearing the *OnMouseDown variables in HTMLAnchorElement (see bug 10338) on mouseenter and not mouseout. This is because the values of these variables are needed for drag operations but these happen after mouseout events. It shouldn't be necessary to clear these variables at all because they are only accessed on a mouse click or mouse drag event, both of which happen after a mousedown event. But I figure it's best to clear them for safety anyway.

I'm not sure that it is possible to test this automatically, so I've just modified the description for the contenteditable-link.html manual test to describe these changes.

I added the WebElementLinkIsLiveKey to the WebElementDictionary so that WebHTMLView can determine whether or not a link is 'live', and so if it is draggable.
Comment 2 Timothy Hatcher 2006-11-03 16:04:49 PST
Comment on attachment 11128 [details]
patch

We can't modify WebKit/WebView/WebView.h, since the API would need to be reviewed.  Is WebElementLinkIsLiveKey needed to fix this? Also this has some bit rot, WebElementDictionary has moved/changed.
Comment 3 Graham Dennis 2006-11-05 23:15:27 PST
Created attachment 11391 [details]
patch 2

Updated patch fixing patch rot and other issues.

I'm honestly not sure what I was thinking when I changed WebView.h, I should have modified WebViewPrivate.h instead... This patch fixes that and extends WebCore's HitTestResult to provide the info for the WebElementDictionary. As a result, the isLiveLink method doesn't need to be exported by the *.idl file like it was in the previous patch.
Comment 4 Graham Dennis 2006-11-06 14:53:29 PST
Landed in r17629