Community
Participate
Working Groups
Build Identifier: Generally, editors allow you to select text and drag it around within the document. You can also generally drop text into an editor from elsewhere. These both work fine in TextMate and textareas in Firefox. This corresponds to https://bugzilla.mozilla.org/show_bug.cgi?id=687580 in Mozilla's bugzilla. Reproducible: Always Steps to Reproduce: 1. Paste some stuff inside scratchpad. 2. Select a bunch of text, and try to drag it. Scratchpad will try to chaneg the selection instead of supporting dragging. 3. Now try selecting a bunch of text on a web page and dropping it in scratchpad. The drop operation doesn't have any effect.
I have implemented support for drag and drop in Orion. Here is the code: https://github.com/mihaisucan/orion.client/tree/bug-358623 Please review the code and let me know what changes are needed before we can push it into the Orion repo. Implementation details: I used the HTML5 Drag and Drop API to implement DnD support. This works with Firefox and Webkit-based browsers. Users can drag the code out of Orion into other software and web applications. You can also do DnD inside Orion. You can drop code from other apps into Orion as well. Both the copy and move operations are supported. I added a couple of TODOs that will need to be done some day, but I hope they are not high priority. They are mostly about improved UI during dragging and dropping. Please note that for the move operation I had to use compound changes within the UndoStack. I had to do a rather ugly hack to allow the TextView to access the UndoStack. Comments for a less hacky solution are very much welcome. Thank you very much!
Mihai, we are still working on this problem. At this point we believe we should add events to the view and have the implementation in the editor. The major problem with your code is that is assumes that any mouse down within the text selection will start a drag. That is not true. In Windows and GTK there is native APIs for that: http://msdn.microsoft.com/en-us/library/windows/desktop/ms646256%28v=vs.85%29.aspx http://developer.gnome.org/gtk3/stable/gtk3-Drag-and-Drop.html#gtk-drag-check-threshold Try this running with your code: Select a few lines in the editor Just click in the middle of the selection (mouse down, mouse up without mouse move in between). Note that the selection is not clear, worse, the dom selection and the view selection are out of sync. try more clicks or use arrow keys, the behavior is wrong.
Thank you very much Felipe for looking into this bug and into the proposed patch! (In reply to comment #2) > Mihai, we are still working on this problem. > At this point we believe we should add events to the view and have the > implementation in the editor. Why should this belong in the editor? Unfortunately, anything that moves into the editorFeatures.js file is not (easily) reusable at this point. Also, there are features in the editor that we would certainly want to take in, but at the moment we try not to - to avoid adding features that have other sets of bugs. (I should send an email to the mailing list with some thoughts about how editorFeatures.js can be made reusable for us.) Ultimately, this is not a problem, even if DnD support is added into the editor (not into the TextView), because we can make editorFeatures.js reusable. > The major problem with your code is that is assumes that any mouse down within > the text selection will start a drag. That is not true. That's an unfortunate case that I missed. Isn't this something I can fix? I should be able to do that. > In Windows and GTK there is native APIs for that: > http://msdn.microsoft.com/en-us/library/windows/desktop/ms646256%28v=vs.85%29.aspx > http://developer.gnome.org/gtk3/stable/gtk3-Drag-and-Drop.html#gtk-drag-check-threshold > > Try this running with your code: > > Select a few lines in the editor > Just click in the middle of the selection (mouse down, mouse up without mouse > move in between). > > Note that the selection is not clear, > worse, the dom selection and the view selection are out of sync. > try more clicks or use arrow keys, the behavior is wrong. Thanks for the explanation and STR. I will look into fixing this issue.
(In reply to comment #3) > > Why should this belong in the editor? Basically because the editor can see the undoStack, and the view can't. > > Unfortunately, anything that moves into the editorFeatures.js file is not > (easily) reusable at this point. Also, there are features in the editor that we > would certainly want to take in, but at the moment we try not to - to avoid > adding features that have other sets of bugs. What about editor.js ? I guess the question is, are you use the view + editor + your custom editorFeatures, or are you just using the view from us ? > > (I should send an email to the mailing list with some thoughts about how > editorFeatures.js can be made reusable for us.) > Yes, I would like to understand that. > Ultimately, this is not a problem, even if DnD support is added into the editor > (not into the TextView), because we can make editorFeatures.js reusable. > > > > The major problem with your code is that is assumes that any mouse down within > > the text selection will start a drag. That is not true. > > That's an unfortunate case that I missed. Isn't this something I can fix? I > should be able to do that. > > Silenio and I are working on this right now. The idea is to let the mouse down go to the user agent (when button===1 and click inside the selection), but then do the work (the otherwise would be done in mouse up) in the mouse up handler if (and only if) a drag start is not sent. Give us a bit more time and I'll upload a patch.
Made a fix for the STR you provided. Now simply clicking on the selection won't leave the editor in a broken state. Fix: https://github.com/mihaisucan/orion.client/commit/75098132390c45e7b8326fab5ec74a2863919fc2 Another bug I found was dragging the selection onto itself caused problems. Fix: https://github.com/mihaisucan/orion.client/commit/f6fc1a8dac71112c845f5b6c75773f83000d8b94 Thanks again for your investigation!
(In reply to comment #4) > (In reply to comment #3) > > > > Why should this belong in the editor? > > Basically because the editor can see the undoStack, and the view can't. Good point. > > > > Unfortunately, anything that moves into the editorFeatures.js file is not > > (easily) reusable at this point. Also, there are features in the editor that we > > would certainly want to take in, but at the moment we try not to - to avoid > > adding features that have other sets of bugs. > > What about editor.js ? At the time when I last looked at the script it had too many choices pre-made. > I guess the question is, are you use the view + editor + your custom > editorFeatures, > or are you just using the view from us ? We only use the TextView at the moment. This is the list of files: - /orion/textview/keyBinding.js - /orion/textview/rulers.js - /orion/textview/undoStack.js - /orion/textview/textModel.js - /orion/textview/tooltip.js - /orion/textview/textView.js - /orion/editor/htmlGrammar.js - /orion/editor/textMateStyler.js - /examples/textview/textStyler.js They are concatenated into a single orion.js that we ship in Firefox. > > Ultimately, this is not a problem, even if DnD support is added into the editor > > (not into the TextView), because we can make editorFeatures.js reusable. > > > > > > > The major problem with your code is that is assumes that any mouse down within > > > the text selection will start a drag. That is not true. > > > > That's an unfortunate case that I missed. Isn't this something I can fix? I > > should be able to do that. > > > > > > Silenio and I are working on this right now. > The idea is to let the mouse down go to the user agent (when button===1 and > click inside the selection), but then do the work (the otherwise would be done > in mouse up) in the mouse up handler if (and only if) a drag start is not sent. > > Give us a bit more time and I'll upload a patch. That's great! But there seems to have been a mid-air collision, sorry! I already pushed a small fix to my repo, but no problem. Looking forward to see your upload! Thank you!
Just a quick note relevant to this bug: today I started to experiment with integrating editor.js and editorFeatures.js into Firefox. This is just for experimental purposes. This allows me to soon open bug reports about changes we want in those files, to be able to land those files into Firefox. The relevance of this activity is that DnD can land in the editor-side of Orion. I would no longer make it a requirement for this DnD support to be in the TextView-side of things. For the Firefox 10 aurora merge we might decide to include my TextView patch for DnD support. So, after Fx10 we will probably focus on editor.js and editorFeatures.js integration (which would also give us the upstream DnD support, along with other upstream features). Either way, we would very much appreciate for DnD support to land in Orion as soon as possible. (we certainly want to avoid keeping our Orion patched) Thank you very much!
Hi Mihai, my current thinking is put all the DnD support in a new file (dragdrop.js), this will be a new object that can be created passing the view and the undostack (basemodel maybe) and it handles all the drag&drop work. This way it can be consumed by the demo, editor and firefox the same way. One reason for doing this way is that people who do not want drag and drop should not need to download more code for something that is not needed. It also helps us to keep textview.js smaller. Right now Silenio and I are working to add all the drag drop events to the view (along with all the intrinsic events), but before we do that we are changing our events (again) to comply with W3C. There is some discussion about that in Bug 349957 (and a bit in Bug 334583) We would like to land the event support today and add the drag support early next week.
Hello Felipe! (In reply to comment #8) > Hi Mihai, > > my current thinking is put all the DnD support in a new file (dragdrop.js), > this will be a new object that can be created passing the view and the > undostack (basemodel maybe) and it handles all the drag&drop work. > > This way it can be consumed by the demo, editor and firefox the same way. > > One reason for doing this way is that people who do not want drag and drop > should not need to download more code for something that is not needed. It also > helps us to keep textview.js smaller. Sounds good! > Right now Silenio and I are working to add all the drag drop events to the view > (along with all the intrinsic events), but before we do that we are changing > our events (again) to comply with W3C. There is some discussion about that in > Bug 349957 (and a bit in Bug 334583) > > We would like to land the event support today and add the drag support early > next week. Great! I am looking forward for the patches to land. Thank you very much!
We released all the work we did this week, see: http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=fb470b9cb9bf93e715b463fc89ad10010093fd29 It includes: - add dragstart, drag, dragend, dragenter, dragover, dragleave, drop events to the TextView - the necessary logic in Textview (when to allow/prevent the dom events) for the events - a new object that implements these events (see textDND.js) - support the our demo application There are few limitation, but we can say that drag and drop is working for the demo in Chrome/IE/Firefox. Here is list of outstanding issues - https://bugzilla.mozilla.org/show_bug.cgi?id=614974 This means that drop always gets the visible selected text, for example, select all and drag the text to another application. Only the visible text is dropped. Other problem caused by this bug is that white spaces at the begging of the lines are lost. - https://bugzilla.mozilla.org/show_bug.cgi?id=701122 Click and drag in the first char of selection, the view will expect a drag that will never come. It fixes itself during mouse up. - No insertion mark in Chrome, need to emulated it during dragOver - Insertion mark at the wrong place at the end of line (IE/FF), this is due to the trailing white space the view adds to the end of every line. - no auto scrolling - can be implemented during dragOver (I think) - drop effect does not work everywhere, some platforms is always move, other always copy. This bug is more visible when interacting with external applications.
I found a bad bug testing this morning, start a drag operation and cancel it with the escape key. Mouse selection stops working on all browsers.
(In reply to comment #11) > I found a bad bug testing this morning, start a drag operation and cancel it > with the escape key. Mouse selection stops working on all browsers. The dropTarget flag was not reset properly when the event was not hooked. Note that Firefox has further problems: the caret does not show after the user cancels DND. Fixed http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/bundles/org.eclipse.orion.client.editor/web/orion/textview/textView.js?id=6a6c33db3c109e821ad8f01feac98279dfc0a96d
(In reply to comment #10) > We released all the work we did this week, see: > http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=fb470b9cb9bf93e715b463fc89ad10010093fd29 > > It includes: > [...] Thank you very much for your work! Very much appreciated! > There are few limitation, but we can say that drag and drop is working for the > demo in Chrome/IE/Firefox. > > Here is list of outstanding issues > - https://bugzilla.mozilla.org/show_bug.cgi?id=614974 > This means that drop always gets the visible selected text, for example, select > all and drag the text to another application. Only the visible text is dropped. > Other problem caused by this bug is that white spaces at the begging of the > lines are lost. Wow and ouch. Ugly bug. My implementation worked because I was dragging the overlayDiv. Eh. I will ping people about a possible fix for our Mozilla Bug 614974. > - https://bugzilla.mozilla.org/show_bug.cgi?id=701122 > Click and drag in the first char of selection, the view will expect a drag that > will never come. It fixes itself during mouse up. Ugly, but I hope we can live with it. > - no auto scrolling - can be implemented during dragOver (I think) True. The dragover event handler can get the event coords, convert to offsets and do auto scrolling fun... > - drop effect does not work everywhere, some platforms is always move, other > always copy. This bug is more visible when interacting with external > applications. Ah, known. This is not a problem with the implementation. Some apps only allow copy, some only move. No problem. (In reply to comment #12) > (In reply to comment #11) > > I found a bad bug testing this morning, start a drag operation and cancel it > > with the escape key. Mouse selection stops working on all browsers. > > > The dropTarget flag was not reset properly when the event was not hooked. Note > that Firefox has further problems: the caret does not show after the user > cancels DND. > > Fixed > http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/bundles/org.eclipse.orion.client.editor/web/orion/textview/textView.js?id=6a6c33db3c109e821ad8f01feac98279dfc0a96d Thank you! The caret not showing is something that you might be able to fix with a blur() + focus() call. Did you try this? (from dragend - afaik the event is fired even when the drag is canceled)
Hello guys! Today I worked on fixes for the new Orion Drag and Drop support, specific to Firefox. Here's the code: https://github.com/mihaisucan/orion.client/tree/bug-358623-2 Explanation: - The Mozilla Bug 614974 is very ugly and we need to fix this, but until then we need a workaround. The code includes a workaround: set contentEditable=false and draggable=true on mousedown, and set them back to their initial states ASAP (on mouseup/post-dragstart, etc). Rewrites are not allowed when contentEditable=true. The 614794 test case and bug description are not quite accurate... - The above code changes also avoid Mozilla Bug 701122. - For drop to work properly we need contentEditable=true, hence this needs to be set to true immediately after dragstart. - Included some more focus()/blur() incantations to avoid the cursor hiding bug when the drag is canceled. It seems the current fix doesn't cover all cases (eg. drag the selection and drop onto itself). Please review and let me know if this patch can land. Thank you! (compared to what we already shipped in Orion, Firefox 10, this DnD support is better in the sense that it has a good drag image generated by the browser and it has the insertion mark.)
We run into some problems fixing Bug 358628 that were worked around but the same strategy used in the patch for this problem (flipping contentEditable flag). So we decided to do both changes together. Implementation Notes: The code (to flip contentEditable) in _handleMouse() was moved to _handlebodyMouseDown() ->We need to code to run when the click occur in the scrollbars (for Bug 358628) (we remove the timer to reset ignoreBlur in_ handleMouse()) the reset code (flip contentEditable back) was added to: 1. _handleBodyMouseUp(), plus fixCaret() 2. _handleDragStart() (in a timer - like you did) Removed the code from _handleDragEnd (since _handleDragStart does the work) But _handleDragEnd() still needs to call fixCaret() (replace the "incantations" code) - It is needed because the mouse up event is not sent. Removed the call to focus() in handleDrop() -> could not find a case where it was necessary Removed the reset code from _handleMouseUp (now done in _handleBodyMouseUp()) Removed the focus() from _handleMouseUp (now done in _handleBodyMouseUp()->fixCaret()) Removed the dragOffset === -2 hack from (dragEnd/mouseUp) since we are always calling fixCaret() from _handleBodyMouseUp.
Fixed: http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=925b8b8408973c9329f324dfef22c697ff505ddc
Thank you very much for your work here and for landing the workaround for Firefox. In regards to the fixCaret() thing, there's still the case when dragging the selection onto itself leaves the user without a visible caret - in the current Orion master branch, if you try this you will end up without a caret. I don't recall which one of those fixCaret() calls fixed the case - but what I do know is I needed every one of those - tried to keep their number to a minimum.
(In reply to comment #17) > In regards to the fixCaret() thing, there's still the case when dragging the > selection onto itself leaves the user without a visible caret Thanks for pointing this out, here is the fix: http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=61fd44bff613c790a2fb29946be2537c2771eda5
(In reply to comment #18) > (In reply to comment #17) > > In regards to the fixCaret() thing, there's still the case when dragging the > > selection onto itself leaves the user without a visible caret > > Thanks for pointing this out, here is the fix: > http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=61fd44bff613c790a2fb29946be2537c2771eda5 Thank you very much!
