The default bug view has changed. See this FAQ.

[GTK/X11] selecting text in scratchpad doesn't place it on the X primary selection

RESOLVED FIXED in Firefox 11

Status

()

Firefox
Developer Tools
P2
normal
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: dbaron, Assigned: msucan)

Tracking

Trunk
Firefox 11
All
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sourceeditor][orion][fixed-in-fx-team])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
On the Linux/X11 desktop (definitely in GNOME apps, and I think in KDE as well), selecting text should make that text the X primary selection ("PRIMARY"), such that middle clicking in another app taking text input will paste that text.

(This is because X has a longstanding copy/paste model that's somewhat more like the mental model of dragging and dropping text than the Windows/Mac copy/paste model, except using select and middle-click.  The GNOME and KDE desktops have added a more Windows/Mac-like model with Copy and Paste operations in *addition* to that traditional model, but many X users use the traditional model.)

Steps to reproduce:
 1. Tools -> Web Developer -> Scratchpad
 2. select a word of the text in the window that opens
 3. open a terminal
 4. middle click in the terminal

Expected results:  The word that was selected gets pasted in the terminal (just like it does if you select a word of text in the browser or in view source or in some other app).

Actual results: either nothing is pasted or some other text is pasted
(Reporter)

Updated

6 years ago
Blocks: 695035
There are a whole bunch of Gtk+ bugs that, collectively, add up to "I'm amazed you noticed this was broken, because it's already so broken as to be unusable."  Please see

https://bugzilla.gnome.org/show_bug.cgi?id=333514
https://bugzilla.gnome.org/show_bug.cgi?id=334060
https://bugzilla.gnome.org/show_bug.cgi?id=584236

... all of which have been hanging around not getting fixed since 2005-ish.  Of particular interest to fixing the bugs in *our* code, #333514 describes in detail how PRIMARY/middle-mouse is *supposed* to work.  I do not trust the linked fd.o clipboard spec to be correct, however; there were any number of people arguing quite strenuously for incorrect behaviors. :-(
(Assignee)

Comment 2

6 years ago
Zack: thanks for your comment and links, that was very helpful (I read 333514). I have only made a complete switch to Linux about 5 years ago and the Select/middle click behavior is something I am not used to. (I came from Windows.)

The place where we can fix this bug and 695035 is interesting to pick:

1) We can fix it in the SourceEditor component that wraps Orion. Selection events can be used to copy the text to PRIMARY. We can also hook into the middle click event and generate a paste into Orion.

2) We can look into Orion itself why this doesn't work. Orion makes a native selection, that should automatically be copied to PRIMARY (perhaps there's something prevent this from happening?). The middle click event is canceled by Orion. We can make it so the event is not canceled - to allow the paste to happen.
(Assignee)

Comment 3

6 years ago
Bug reported upstream:

https://bugs.eclipse.org/bugs/show_bug.cgi?id=361471
These days, middle-mouse paste and PRIMARY are kinda a hidden easter egg.  Users accustomed to Windows should be able to ignore them completely.  But for people like me, who have been using Unix on the desktop since the nineties, they're very convenient and it drives us nuts when they don't work precisely the way Xt made them work.  Which was largely accidental but happened to be quite ergonomic :)

Updated

6 years ago
Priority: -- → P2
(Assignee)

Comment 5

5 years ago
Results from upstream: this bug cannot be fixed within Orion.

We need to do this from the Source Editor integration code. I have a plan how this can be fixed.
(Assignee)

Comment 6

5 years ago
Created attachment 580452 [details] [diff] [review]
proposed patch

Proposed patch with included test.

Pushed this patch and the dependencies to the try server:

https://tbpl.mozilla.org/?tree=Try&rev=f9802b4377cf
Assignee: nobody → mihai.sucan
Status: NEW → ASSIGNED
Attachment #580452 - Flags: review?(rcampbell)
(Assignee)

Updated

5 years ago
Depends on: 702331
Comment on attachment 580452 [details] [diff] [review]
proposed patch

 
   /**
+   *
+   * Orion Selection event handler for Linux users. This allows one to select
+   * text and have it copied into the X11 PRIMARY

looks like an extra * line in there. (nit)

     Services.prefs.getBoolPref(SourceEditor.PREFS.EXPAND_TAB);
+
+  this._onOrionSelection = this._onOrionSelection.bind(this);

I don't see a corresponding this._onOrionSelection = null or delete segment in destroy. Maybe it's not in this patch.

This looks good though.
Attachment #580452 - Flags: review?(rcampbell) → review+
> +   * Orion Selection event handler for Linux users. This allows one to

Replace "Linux" with "X11" or "X Window System" throughout; the behavior we're trying to preserve here originates with MIT Athena, and has no historical connection to Linux.
(Assignee)

Comment 9

5 years ago
Created attachment 580965 [details] [diff] [review]
[in-fx-team] updated patch

Updated the patch. Thanks Rob for the review! Good catch!

I also made some minor changes to make sure the test passes under more diverse conditions.
Attachment #580452 - Attachment is obsolete: true
(Assignee)

Comment 10

5 years ago
Comment on attachment 580965 [details] [diff] [review]
[in-fx-team] updated patch

https://hg.mozilla.org/integration/fx-team/rev/7cf966da2e93
Attachment #580965 - Attachment description: updated patch → [in-fx-team] updated patch
(Assignee)

Comment 11

5 years ago
Comment on attachment 580965 [details] [diff] [review]
[in-fx-team] updated patch

(copy/paste fail in the previous comment)

correct link:
https://hg.mozilla.org/integration/fx-team/rev/5bd81a413dea
(Assignee)

Updated

5 years ago
Whiteboard: [sourceeditor][orion] → [sourceeditor][orion][fixed-in-fx-team]
Comment on attachment 580965 [details] [diff] [review]
[in-fx-team] updated patch

>+    if (Services.appinfo.OS == "Linux") {
>+      this._view.addEventListener("Selection", this._onOrionSelection);
>+    }

I shoulda actually read the patch before; *this is wrong*.  You need to detect X11, not Linux.  We have a fair number of users on systems that use X11 as their window system but are not Linux.

I'm afraid I don't know how to do this correctly.

>+    if (Services.appinfo.OS == "Linux") {
>+      this._view.removeEventListener("Selection", this._onOrionSelection);
>+    }

Similarly.

I'm r-ing to mark this as a serious concern, but you're welcome to fix as a follow-up patch.
Attachment #580965 - Flags: review-
(Assignee)

Comment 13

5 years ago
(In reply to Zack Weinberg (:zwol) from comment #12)
> Comment on attachment 580965 [details] [diff] [review]
> [in-fx-team] updated patch
> 
> >+    if (Services.appinfo.OS == "Linux") {
> >+      this._view.addEventListener("Selection", this._onOrionSelection);
> >+    }
> 
> I shoulda actually read the patch before; *this is wrong*.  You need to
> detect X11, not Linux.  We have a fair number of users on systems that use
> X11 as their window system but are not Linux.
> 
> I'm afraid I don't know how to do this correctly.
> 
> >+    if (Services.appinfo.OS == "Linux") {
> >+      this._view.removeEventListener("Selection", this._onOrionSelection);
> >+    }
> 
> Similarly.
> 
> I'm r-ing to mark this as a serious concern, but you're welcome to fix as a
> follow-up patch.

Sorry about this. Who can I ping on how I can detect X11 on the system?

Or should I not special case this? Can we use the selection primary on all systems?

I suggest we fix this in a follow up bug/patch.
(In reply to Mihai Sucan [:msucan] from comment #13)
> 
> Sorry about this. Who can I ping on how I can detect X11 on the system?

The only person I can think of who isn't already cc:ed but might know is karlt.

> Or should I not special case this? Can we use the selection primary on all
> systems?

Dunno.

> I suggest we fix this in a follow up bug/patch.

Follow-up patch, yes, but I think this bug should not be closed until this is corrected.
Doing a bit of my own homework: unless karlt has a better idea, I suggest the condition

   Services.appinfo.widgetToolkit.slice(0,3) === "gtk"

The slice() is to be future-proof against the addition of a gtk3 back end -- see bug 627699.  (We can worry about Gtk/Wayland later.)  As far as I am aware, the qt widget backend is not for use with KDE, so we don't need that in there.

See http://mxr.mozilla.org/mozilla-central/source/xpcom/system/nsIXULRuntime.idl#88
Can you access CC[NS_NETWORK_PROTOCOL_CONTRACTID_PREFIX + "http"].getService(CI.nsIHttpProtocolHandler)?

Its "platform" property is "X11".
please file a bug to address Zack's concerns as a follow-up. This is in.

https://hg.mozilla.org/mozilla-central/rev/5bd81a413dea
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 11
(In reply to Zack Weinberg (:zwol) from comment #14)
> (In reply to Mihai Sucan [:msucan] from comment #13)
> > 
> > Sorry about this. Who can I ping on how I can detect X11 on the system?
> 
> The only person I can think of who isn't already cc:ed but might know is
> karlt.
> 
> > Or should I not special case this? Can we use the selection primary on all
> > systems?
> 
> Dunno.
> 
> > I suggest we fix this in a follow up bug/patch.
> 
> Follow-up patch, yes, but I think this bug should not be closed until this
> is corrected.

I think you're being a tad overzealous about this. Linux is a supported platform. "Other variants of Unix" is really a fringe thing. While I want to support them too, getting this fix in for the majority of our *nix users is acceptable for a developer tool.
You need to log in before you can comment on or make changes to this bug.