Bug 28632 – Unable to drag Keepvid.com bookmarklet (to bookmarks bar)

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[jasneet]

I Steps:
Go to http://keepvid.com/
Try to drag the Keepvid bookmarklet (to the bookmarks bar)

II Issue:
Dragging is not possible

III Other Browsers:
IE8: not ok
FF3: ok

IV Nightly tested: 46809

Bug in Chromium : http://code.google.com/p/chromium/issues/detail?id=16307

[Victor Wang]

The reason why this bookmark can't be dragged is because it has child node. The current webkit code does not allow a node to be dragged if it has child node:
bool EventHandler::shouldDragAutoNode(Node* node, const IntPoint& point) const
{
    if (!node || node->hasChildNodes() || !m_frame->view())
        return false;
    ...
}

[Victor Wang]

Hi David,

In this case, the node to drag is Anchor node, which has a #text child node. The EventHandler::shouldDragAutoNode in WebCore\page\EvnetHandler.cpp return false if the node to drag has child nodes, do you know any background why this check is required? Is it OK to remove the child node checking to fix this bug? If not, any suggestions on how to fix it?

This is a bug in both Safari and Chrome.

Thanks,
Victor

[Dave Hyatt]

Not sure why that check is there.  Oliver Hunt (olliej) in IRC might remember.

[Dave Hyatt]

Does this code actually have to do with link dragging or is it more about dragging of text... most links have child nodes obviously and still work in Safari, so it's not clear to me if you're looking in the right spot?

[Oliver Hunt]

What's the backtrace that leads to EventHandler::shouldDragAutoNode in this case?

[Victor Wang]

I think normally when you drag a link, it is the child text node getting dragged so it works fine for link dragging. "Keep it" bookmark is little special, the actual text is indented -10000px, so you are not actually dragging the text link. Instead, you are dragging the image part, which corresponds to the anchor node. I believe this is why it fails but other link dragging works fine. I tested it by removing the child node checking and it works after that.

[Victor Wang]

WebCore::EventHandler::shouldDragAutoNode(WebCore::Node * node=0x048d9140, const WebCore::IntPoint & point={...}) Line 2170	C++
WebCore::RenderObject::draggableNode(bool dhtmlOK=true, bool uaOK=true, int x=456, int y=41, bool & dhtmlWillDrag=false) Line 1404 + 0x4c bytes	C++
WebCore::EventHandler::handleDrag(const WebCore::MouseEventWithHitTestResults & event={...}) Line 2233 + 0x48 bytes	C++
WebCore::EventHandler::handleMouseDraggedEvent(const WebCore::MouseEventWithHitTestResults & event={...}) Line 429 + 0xc bytes	C++
WebCore::EventHandler::handleMouseMoveEvent(const WebCore::PlatformMouseEvent & mouseEvent={...}, WebCore::HitTestResult * hoveredNode=0x0438ecac) Line 1413 + 0xc bytes	C++
WebCore::EventHandler::mouseMoved(const WebCore::PlatformMouseEvent & event={...}) Line 1309 + 0x10 bytes	C++
WebViewImpl::MouseMove(const WebKit::WebMouseEvent & event={...}) Line 448	C++
WebViewImpl::handleInputEvent(const WebKit::WebInputEvent & input_event={...}) Line 1041	C++
RenderWidget::OnHandleInputEvent(const IPC::Message & message={...}) Line 318 + 0x1b bytes	C++

[Oliver Hunt]

Yeah i think the hasChildNodes check should just go away

[Victor Wang]

Created attachment 40450 [details]
Proposed patch

[WebKit Commit Bot]

Comment on attachment 40450 [details]
Proposed patch

Clearing flags on attachment: 40450

Committed r48981: <http://trac.webkit.org/changeset/48981>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

This change broke the following case:
<img style="-webkit-user-drag: none;" src="foo.png">
and made the image draggable.

(In reply to comment #12)
> This change broke the following case:
> <img style="-webkit-user-drag: none;" src="foo.png">
> and made the image draggable.

Filed as bug 35459.

[Victor Wang]

working on it. thanks!