hello,

i can't reproduce this bug with the same workflow on a 
date debian stretch (gnome 3.22.2).

(In reply to khatar from comment #1)
> hello,
> 
> i can't reproduce this bug with the same workflow on a 
> date debian stretch (gnome 3.22.2).

Because you're using X11 I'm guessing.

I can reproduce as well on GNOME 3.24 (gnome-characters 3.22.0).

I am on Wayland, and it happens only sporadically.  Interestingly, it doesn't work if I select a Unicode symbol (e.g. "Thumbs Up Sign") at first, while it starts working once I select an ASCII symbol (e.g. "Asterisk").

I use Characters quite often so this bug is very annoying for me. I can reproduce this everytime, I have to open the app to copy the characters I want. Also, when clicking the Characters icon in the search results nothing happens. Before it opened the Characters app with more search results (I think).

I have no idea how to fix this; the application and the search provider simply call GtkClipboard#set_text() as follows:

GtkClipboard *clipboard;
clipboard = gtk_clipboard_get (GDK_SELECTION_CLIPBOARD);
clipboard.set_text(text, -1);

https://git.gnome.org/browse/gnome-characters/tree/lib/gc.c#n1132
https://git.gnome.org/browse/gnome-characters/tree/src/searchProvider.js#n117
https://git.gnome.org/browse/gnome-characters/tree/src/character.js#n183

Reassigning to gtk+ for feedback from experts.

Could it be linked to bug 759626 ?

It's not just that. On wayland a surface from the application must be focused before the application can modify/peek the clipboard or any other selection.

In this case there's no application windows at all, thus there's no surface, thus there's no clipboard.

Maybe the copy-to-clipboard feature has to move to the shell side ?

Created attachment 350888 [details] [review]
patch to gnome-characters

So maybe like this?  It seems to work, though quite hacky.

*very* hacky. And it wouldn't work with gnome-characters sandboxed. I think that the search provider needs to tell gnome-shell, through an extension of the protocol, that it's supposed to copy "this" when the search result is activated.

Tangentially, Florian, won't this particular call be a security problem if applications can talk to the shell for their search providers?

(In reply to Bastien Nocera from comment #10)
> *very* hacky. And it wouldn't work with gnome-characters sandboxed. I think
> that the search provider needs to tell gnome-shell, through an extension of
> the protocol, that it's supposed to copy "this" when the search result is
> activated.

That sounds like a cleaner approach, but changing the protocol just for this particular issue seems overkill to me, as I don't see any other use-case of this feature.

Perhaps gnome-shell could simply copy the search result to the clipboard depending on the name or configuration of the search provider?

Created attachment 350958 [details] [review]
remoteSearch: respect "CopyResult" in desktop file

Some search providers such as GNOME Characters want to copy the search
results to a clipboard.  However, on Wayland, GtkClipboard is functional
only if the app has any visible surface.

This patch adds a new keyword "CopyResult" in seach providers' .ini
file.  If the keyword is set to true, the shell by itself copies the
result to the clipboard.

(In reply to Bastien Nocera from comment #10)
> Tangentially, Florian, won't this particular call be a security problem if
> applications can talk to the shell for their search providers?

Yeah, it's an issue when apps are allowed to talk to any name owned by the shell. It's not something apps are expected to do (including for search providers, which actually work the other way round, i.e. the shell talking to the app)), in particular when sandboxed.

Still, we may want to consider turning it off by default ('development-tools' in org.gnome.shell) ...

Review of attachment 350958 [details] [review]:

The commit message is poorly worded - "respect foo property" sounds like there was a well-established field that we failed to support, rather than something newly made-up. And of course a search provider keyfile is not a .desktop file ...

::: js/ui/search.js
@@ +184,2 @@
     _activateResult: function(result, id) {
         this.provider.activateResult(id, this._terms);

Does this make sense, or should we assume that if copyResult is set, then the app uses the activate-copies pattern?

@@ +185,3 @@
         this.provider.activateResult(id, this._terms);
+        if (this.provider.copyResult)
+            this._clipboard.set_text(St.ClipboardType.CLIPBOARD, id);

Of course this doesn't copy the result, but its ID. While I don't know about any other search provider that uses the activate-copies pattern, this looks needlessly restrictive.

Maybe something like:

  activateResult: function(id, terms) {
      if (this.copyResult)
          this.proxy.GetResultClipboardTextRemote(id, (text, error) => {
              this._clipboard.set_text(St.ClipboardType.CLIPBOARD, text);
          });
      else
          this.proxy.ActivateResultRemote(id, terms, global.get_current_time());
  },

Created attachment 351592 [details] [review]
search: copy result to clipboard if requested

Some search providers such as GNOME Characters want to copy search
results to clipboard.  However, on Wayland, clipboards are only
accessible from applications that have a visible surface on display.

This patch allows a search provider to request the shell to copy a
search result to clipboard when 'clipboardText' is included in the meta
of the result.

(In reply to Florian Müllner from comment #14)
> Review of attachment 350958 [details] [review] [review]:
> 
> The commit message is poorly worded - "respect foo property" sounds like
> there was a well-established field that we failed to support, rather than
> something newly made-up. And of course a search provider keyfile is not a
> .desktop file ...

Thank you for the review.

> ::: js/ui/search.js
> @@ +184,2 @@
>      _activateResult: function(result, id) {
>          this.provider.activateResult(id, this._terms);
> 
> Does this make sense, or should we assume that if copyResult is set, then
> the app uses the activate-copies pattern?

I think it still makes sense to call activateResult, even if a search provider wants to copy the result to clipboard, because it allows the search provider to track the activated result as "recently used".

> @@ +185,3 @@
>          this.provider.activateResult(id, this._terms);
> +        if (this.provider.copyResult)
> +            this._clipboard.set_text(St.ClipboardType.CLIPBOARD, id);
> 
> Of course this doesn't copy the result, but its ID. While I don't know about
> any other search provider that uses the activate-copies pattern, this looks
> needlessly restrictive.
> 
> Maybe something like:
> 
>   activateResult: function(id, terms) {
>       if (this.copyResult)
>           this.proxy.GetResultClipboardTextRemote(id, (text, error) => {
>               this._clipboard.set_text(St.ClipboardType.CLIPBOARD, text);
>           });
>       else
>           this.proxy.ActivateResultRemote(id, terms,
> global.get_current_time());
>   },

I guess it could reuse GetResultMetas, which is already called before ActivateResult, instead of adding a new method.  And in that case, we don't even need CopyResult keyword in .ini file anymore, because we can simply check a certain property ("clipboardText" in the patch in comment 15) is set in the result meta.

Review of attachment 351592 [details] [review]:

This looks like a better approach indeed, thanks. Feel free to push to gnome-3-24 as well.

Attachment 351592 [details] pushed as 0142fae - search: copy result to clipboard if requested