| Summary: | [GTK/X11] selecting text in Orion doesn't place it on the X primary selection | ||
|---|---|---|---|
| Product: | [ECD] Orion | Reporter: | Mihai Sucan <mihai.sucan> |
| Component: | Editor | Assignee: | Project Inbox <orion.editor-inbox> |
| Status: | RESOLVED WONTFIX | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | eclipse.felipe, mihai.sucan, Silenio_Quarti, zackw |
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | Linux | ||
| Whiteboard: | |||
|
Description
Mihai Sucan
We StyledText this using:
void handleKeyUp(Event event) {
if (clipboardSelection != null) {
if (clipboardSelection.x != selection.x || clipboardSelection.y != selection.y) {
copySelection(DND.SELECTION_CLIPBOARD);
}
}
clipboardSelection = null;
...
}
void handleKeyDown(Event event) {
if (clipboardSelection == null) {
clipboardSelection = new Point(selection.x, selection.y);
}
...
}
and
void handleMouseUp(Event event) {
if (event.button == 1) {
copySelection(DND.SELECTION_CLIPBOARD);
}
...
}
Setting the selected text to the clipboard all the time was a performance issue, specially during mouse move with a large amount of the text selected.
I guess here we have another problem. We can't access the clipboard most of the time. and even when you can access the clipboard, we can't identifies which clipboard to use. likewise https://bugs.eclipse.org/bugs/show_bug.cgi?id=361474#c2 provided the text being selected is visible in the client area shouldn't we be getting this for free from the contentEditable ? I understand if this fails for select all where most the content is not visible, but when the entire selection is visible I would expect contentEditable to just work... The X11 specification has a lot of clipboards, but there are only two that should normally get used: CLIPBOARD and PRIMARY. And the rules are really simple: 1) Whenever the user explicitly selects text by any method (but *not* when text becomes selected as a side effect of some other action, e.g. keyboard form traversal), claim the PRIMARY clipboard with the contents of that selection. 2) Whenever the user explicitly cuts or copies text, claim *both* PRIMARY and CLIPBOARD with the contents of whatever was cut or copied. 3) Whenever you *lose* the PRIMARY clipboard, cancel the visible selection. 4) But, DO NOT drop the PRIMARY clipboard just because user action has caused the visible selection to be cancelled. It's supposed to persist until a new PRIMARY is asserted. 5) Middle-click pastes the PRIMARY clipboard. 6) Ctrl-V etc paste the CLIPBOARD clipboard. (Yes, it is a consequence of rule #3 that there can be at most one visible selection on the entire desktop at any given time. This is the Right Thing. If you have more than one visible selection users will be confused about what middle-click will paste.) See https://bugzilla.gnome.org/show_bug.cgi?id=333514 for more details. (In reply to comment #4) > The X11 specification has a lot of clipboards, but there are only two that > should normally get used: CLIPBOARD and PRIMARY. And the rules are really > simple: > > [...] Zack, thanks for your explanation, but this bug is more about how we can implement this in Orion, rather than the exact behavior. Being a web app Orion has technical hurdles to overcome before we can implement this feature. (In reply to comment #3) > likewise https://bugs.eclipse.org/bugs/show_bug.cgi?id=361474#c2 > > provided the text being selected is visible in the client area > shouldn't we be getting this for free from the contentEditable ? > I understand if this fails for select all where most the content is not > visible, but when the entire selection is visible I would expect > contentEditable to just work... Felipe, that's what I was thinking of as well. The browser should set the PRIMARY clipboard by itself, natively, without Orion intervention. We can't directly access the primary clipboard from web APIs, but we could allow the native behavior. Still, Orion currently calls preventDefault() for all selection events. Not sure how we could bend things to have our behavior and part of the native behavior? This needs more investigation. (In reply to comment #5) > Felipe, that's what I was thinking of as well. The browser should set the > PRIMARY clipboard by itself, natively, without Orion intervention. I tried on Firefox and Chrome (on Linux) On chrome the paste event is sent and operation works fine. On Firefox the paste event is not sent. both the Textview lets the mouse down event be propagated to the ua. Next test is to try a plain content editable element on Firefox, if the primary selection does not work there then we can close this bug as not Eclipse. Agreed ? (In reply to comment #6) > (In reply to comment #5) > > Felipe, that's what I was thinking of as well. The browser should set the > > PRIMARY clipboard by itself, natively, without Orion intervention. > > I tried on Firefox and Chrome (on Linux) > On chrome the paste event is sent and operation works fine. > On Firefox the paste event is not sent. > both the Textview lets the mouse down event be propagated to the ua. > > > Next test is to try a plain content editable element on Firefox, if the primary > selection does not work there then we can close this bug as not Eclipse. Agreed > ? Please test with a plain simple contentEditable and see if the native paste event happens when the user middle-clicks. If not, we can fix this on Mozilla's side (in two ways, at least). And yes, you can close bug 361474 if the bug can't be fixed in Orion. (I believe you wanted to post the comment there. :) ) More thoughts: - if the paste event happens in Chrome (and perhaps Opera?) then allow it, so at least Orion works correctly in those browsers. - when we get the paste event working in Firefox, for middle click, then it will work with Orion as well. (hopefully) Thank you for your work! Works fine for contentEditable data:text/html,<div style="height:300px;width:300px;border: 1px solid black;" contentEditable="true"> We are doing something in TextView that breaks the native behaviour. More investigation is needed. (In reply to comment #8) > More investigation is needed. Found the culprit, it is the overlayDiv. If remove it then primary selection works (as long as the selection in entirely visible). (In reply to comment #9) > Found the culprit, it is the overlayDiv. > If remove it then primary selection works (as long as the selection in entirely > visible). Not true. I had too many hacks in my code (sorry). The overlayDiv indeed causes problems (as it doesn't allow the clientDiv underneath to process mouse events). But for this to work the TextView would need to allow the default handler for all mouse and keyboard events to run. This is a problem for us. Tested with the latest Orion (now that overlayDiv has been removed). Selecting text won't copy the code into the PRIMARY selection clipboard. If I remove the call to e.preventDefault() from the _handleMouseDown() method, the code is copied into the PRIMARY selection clipboard. Except that the whitespace is lost (just like in the case of drag and drop). The fix would be to switch to PRE elements for each line. If we remove the call to e.preventDefault() we get selection problems (as I encountered them in bug 361474 comment 5 when I worked on that WIP patch. Thoughts? Is this bug fixable? I am starting to doubt it is fixable from Orion... (I expect I can fix it within our Firefox integration code. I hope :) ) We are calling preventDefault() in mouse down to stop the user-agent from selecting text during the mouse move. The overlayDiv was there for the same reason. Mihai, do you know of any other way to do that ? For example, on IE, we stopped the selectstart event in order to do that. But note that changing the mouse down will not solve the problem entirely. The problem will still happen when the selection is set using the keyboard. Interesting that chrome works, what probably tell us that chrome is setting the primary when the selection is set (window.getSelection().addRange() etc), while firefox is using events... (just guessing). Either way, I don't see how to fix this problem. Maybe we will need to see exactly how firefox is implementing primary clipboard support and try to find out if there is anything we can do to get it to work. (In reply to comment #12) > We are calling preventDefault() in mouse down to stop the user-agent from > selecting text during the mouse move. The overlayDiv was there for the same > reason. Yep. > Mihai, do you know of any other way to do that ? For example, on IE, we stopped > the selectstart event in order to do that. You can prevent selection from happening by using -moz-user-select: none (from CSS). But that disables selection entirely - even selection added with the selection API. I tried it myself, but it didn't work... > But note that changing the mouse down will not solve the problem entirely. The > problem will still happen when the selection is set using the keyboard. > > Interesting that chrome works, what probably tell us that chrome is setting the > primary when the selection is set (window.getSelection().addRange() etc), while > firefox is using events... (just guessing). > > Either way, I don't see how to fix this problem. Maybe we will need to see > exactly how firefox is implementing primary clipboard support and try to find > out if there is anything we can do to get it to work. At the moment I am ready to "give up" on this bug here. I don't see how we could allow selection to work while we cancel the mouse events. Also, we can't give up canceling the events. Once we'd get native selection to work, it would still be half-baked: only the visible lines will be copied to the primary clipboard. As far as I know, from web-based code you cannot access the primary clipboard APIs. I will have to look into our clipboard APIs for chrome code and I will implement something that will work on top of the Orion selection events. That will work with mouse and keyboard selections, and it will also copy lines that are not visible within the view. Thank you very much for all the investigation here! (maybe we shouldn't close this bug, in the future there will probably be a way to fix this inside Orion) Just a quick note. This bug has been fixed in the Firefox integration of Orion. For the curious, the code is here: https://bugzilla.mozilla.org/show_bug.cgi?id=695032 (In reply to comment #14) > Just a quick note. This bug has been fixed in the Firefox integration of Orion. > For the curious, the code is here: > > https://bugzilla.mozilla.org/show_bug.cgi?id=695032 It set the clipboard with the selected text every selection change event, right ? For us, in the past (SWT) that was a performance hit. The case was, in large file, the user click (holding the mosue down) and move the mouse out of the client area to the bottom so the editor start to scroll down selecting more text, at some point the selected text becomes very large plus the fast rate of mouse move events will eventually bring the system down to its knee. Holding Shift+page_down in the first line of a large file (with thousands of lines) should also be a problem. (In reply to comment #15) > (In reply to comment #14) > > Just a quick note. This bug has been fixed in the Firefox integration of Orion. > > For the curious, the code is here: > > > > https://bugzilla.mozilla.org/show_bug.cgi?id=695032 > > It set the clipboard with the selected text every selection change event, right > ? Yes. > For us, in the past (SWT) that was a performance hit. > The case was, in large file, the user click (holding the mosue down) and move > the mouse out of the client area to the bottom so the editor start to scroll > down selecting more text, at some point the selected text becomes very large > plus the fast rate of mouse move events will eventually bring the system down > to its knee. > Holding Shift+page_down in the first line of a large file (with thousands of > lines) should also be a problem. Good point. Thank you for pointing out the problem. I will write a follow up patch to fix such cases. Closing as part of a mass clean up of inactive bugs. Please reopen if this problem still occurs or is relevant to you. For more details see: https://dev.eclipse.org/mhonarc/lists/orion-dev/msg03444.html |