Summary: | drop event isn't fired for contentEditable in edit drag | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alfonso MartÃnez de Lizarrondo <amla70> | ||||||||
Component: | HTML Editing | Assignee: | Ryosuke Niwa <rniwa> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, annevk, ayg, darin, dbates, dcheng, enrica, eric, ian, mitz, oliver, rniwa, tony | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 9915, 76194, 76198 | ||||||||||
Attachments: |
|
Description
Alfonso MartÃnez de Lizarrondo
2011-03-27 09:39:25 PDT
This is causing us to fail http://samples.msdn.microsoft.com/ietestcenter/html5/dragdrop_harness.htm?url=types_attribute Appears this also causes us to fail http://samples.msdn.microsoft.com/ietestcenter/html5/dragdrop/setData_dragstart.htm I suspect most of the IETC DnD tests hit this bug. I think we ought to be firing drop event in this case per current HTML5 spec: http://www.whatwg.org/specs/web-apps/current-work/multipage/dnd.html#drag-and-drop-processing-model Look at the step 2 after the sentence "Otherwise, the drag operation might be a success; run these substeps:"; this step fires drop event and is executed for drag & drop operations within editable regions before DOM mutations are made. It turned out that this is a really hard bug to fix :( The spec requires deletion to happen inside the default event handler of dragend event instead of atomically happening immediately after inserting the dragged content once the drop event is fired. However, inserting the content will "orphan" the original selection and we won't be able to delete the dragged content in some cases; in particular, this happens in the attached case where the dragged content and the destination coincide in the same text node. Recovering the selection after the fact is particularly challenging because when ReplaceSelectionCommand::doApply is called, the frame's selection is changed to that of the drag caret's (i.e. destination's). Even if we had stored indexForVisiblePosition, we'll need to adjust the index when the drag source appears after the destination. Created attachment 122342 [details]
work in progress
This regresses undo after drag & drop. I think the current spec is flawed. It's extremely hard for us to split deletion and insertion of contents during drag and still get all editing behaviors (selection, undo, etc...) right. Given that Firefox 9 does it "atomically" in the dragend event, I'm inclined to copy that behavior instead.
Anne would be someone to talk to re: the spec flaws. :) You should also consider filing a bug with the w3c explaining your disagreement. Created attachment 122366 [details]
fixes the bug
There are so many bugs around this feature and I wasn't able to fix them all but I think this is a good step forward. Comment on attachment 122366 [details] fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=122366&action=review This was not nearly as scary as it looked at first. I'm not an expert here. Please let me know if you'd prefer to have someone more expert in this area review your patch. > Source/WebCore/page/EventHandler.cpp:1915 > + bool result = false; Should this be called "preventedDefault" rather than "result" ? Comment on attachment 122366 [details] fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=122366&action=review >> Source/WebCore/page/EventHandler.cpp:1915 >> + bool result = false; > > Should this be called "preventedDefault" rather than "result" ? Will fix before landing. Committed r105396: <http://trac.webkit.org/changeset/105396> (In reply to comment #11) > Committed r105396: <http://trac.webkit.org/changeset/105396> This caused bug 79443. |