[Wayland] Copy to clipboard fails sometimes

RESOLVED FIXED in Firefox 62

Status

()

P3
normal
RESOLVED FIXED
a year ago
10 months ago

People

(Reporter: stransky, Assigned: evan)

Tracking

(Blocks: 1 bug)

Trunk
mozilla62
Points:
---

Firefox Tracking Flags

(firefox-esr60 wontfix, firefox60 wontfix, firefox61 wontfix, firefox62 fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

a year ago
When pasting to clipboard (select text, Menu -> Paste) is sometimes fails to get to the clipboard. Clipboard still contains old content.

Pasting from HTML (dxr for instance) makes it more reproducible.
(Reporter)

Comment 1

a year ago
It also applies to X11 selection clipboard emulated by Gtk+.
(Reporter)

Comment 2

a year ago
Looks like the Firefox Wayland clipboard is somehow isolated from the system one when it happens. When it works when copy -> paste between Firefox windows but the clipboard content is not propagated outside the browser.
Summary: [Wayland] Paste to clipboard fails sometimes → [Wayland] Copy to clipboard fails sometimes

Comment 3

a year ago
I confirm that I'm having clipboard issues under firefox-wayland as well. I'm running FF59.0b13 at the moment build with the: ac_add_options --enable-default-toolkit=cairo-gtk3-wayland switch. 

The clipboard is constantly being emptied by FF-wayland. I have not noted this issue under FF-X11 but it is crippling under Wayland.

Say I copy something in a form or text-field, it will be gone from the clipboard by them time I click in the url bar or switch to a text manager.

From what I observed from Gnome Wayland's clipboard: if text was copied to clipboard while you were in instance A, you can still paste content in instance B as long as A is still open. As soon as you close A, clipboard gets flushed if A was the last one to be copied from. 

It seems to me that FF-wayland is somehow constantly and in-advertantly triggering wayland's clipboard flush mechanism or maybe it's just outright crashing it. I don't know. 

For anyone that's interested, as temporary fix, I wrote a little bash script to repopulate the clipboard. It needs xclip to work. It basically pasted content to file. Checks if the file is empty. If it's not empty, it backs clipboard content up, if it is empty, it restores backed up content to clipboard.  Feel free to use it: https://pastebin.com/r3m94uDW

Comment 4

a year ago
Using my script previously mention script (https://pastebin.com/r3m94uDW) which has a debug output for what's happening in the clipboard, I've noted that the content of the clipboard remains intact in Firefox until I hit ctrl-v or select Paste at which point, my script notices that the clipboard is empty.

something in the pasting process is crashing or flushing the clipboard.
(Assignee)

Comment 5

11 months ago
I cloned mozilla-central today and built Firefox with wayland support. In my local build if I copy text and paste it, 100% of the time the paste operation fails (no output). If I then paste in another GTK Wayland application (e.g. gedit) the content I copied is indeed pasted. So my experience is consistent with comment #4, i.e. the issue seems to be on the paste side not the copy side.

Another interesting data point: in my local build, if I try to paste text into chats on hangouts.google.com Firefox crashes with a segfault. It seems very likely to me that this is related.
(Assignee)

Comment 6

11 months ago
This is what the backtrace looks like when I paste text on google hangouts:

(gdb) bt
#0  0x00007f2c074c9b80 in __GI___nanosleep (requested_time=requested_time@entry=0x7ffdbef64f20, remaining=remaining@entry=0x7ffdbef64f20) at ../sysdeps/unix/sysv/linux/nanosleep.c:28
#1  0x00007f2c074c9a5a in __sleep (seconds=0) at ../sysdeps/posix/sleep.c:55
#2  0x00007f2bf924e4f8 in ah_crap_handler(int) (signum=11) at /home/evan/code/mozilla-central/toolkit/xre/nsSigHandlers.cpp:102
#3  0x00007f2bf923f23c in nsProfileLock::FatalSignalHandler(int, siginfo_t*, void*) (signo=11, info=0x7ffdbef651f0, context=0x7ffdbef650c0) at /home/evan/code/mozilla-central/toolkit/profile/nsProfileLock.cpp:187
#4  0x00007f2bf99e4ded in WasmFaultHandler(int, siginfo_t*, void*) (signum=11, info=0x7ffdbef651f0, context=0x7ffdbef650c0) at /home/evan/code/mozilla-central/js/src/wasm/WasmSignalHandlers.cpp:1362
#5  0x00007f2c08315fb0 in <signal handler called> () at /lib64/libpthread.so.0
#6  0x00007f2bf856de76 in memcpy (__len=4294967295, __src=0x0, __dest=0x7f2a8b400000) at /usr/include/bits/string_fortified.h:34
#7  0x00007f2bf856de76 in nsRetrievalContextWayland::TransferFastTrackClipboard(int, _GtkSelectionData*) (this=0x7f2bddf3b430, aClipboardRequestNumber=<optimized out>, aSelectionData=0x7ffdbef65d20) at /home/evan/code/mozilla-central/widget/gtk/nsClipboardWayland.cpp:674
#8  0x00007f2bf856deb6 in wayland_clipboard_contents_received(GtkClipboard*, GtkSelectionData*, gpointer) (clipboard=<optimized out>, selection_data=selection_data@entry=0x7ffdbef65d20, data=data@entry=0x7f2bbb4e9250) at /home/evan/code/mozilla-central/widget/gtk/nsClipboardWayland.cpp:660
#9  0x00007f2c05180af5 in selection_received (widget=0x7f2bc37732b0 [GtkInvisible], selection_data=0x7ffdbef65d20, time=<optimized out>) at gtkclipboard.c:960
#13 0x00007f2c0225fab4 in <emit signal 0x7f2c05207984 "selection-received" on instance 0x7f2bc37732b0 [GtkInvisible]> (instance=0x7f2bc37732b0, detailed_signal=detailed_signal@entry=0x7f2c05207984 "selection-received") at gsignal.c:3487
    #10 0x00007f2c02242add in g_closure_invoke (closure=0x7f2bc852b440, return_value=0x0, n_param_values=3, param_values=0x7ffdbef659b0, invocation_hint=0x7ffdbef65930) at gclosure.c:804
    #11 0x00007f2c02255eb3 in signal_emit_unlocked_R (node=node@entry=0x7f2be3e2b9a0, detail=detail@entry=0, instance=instance@entry=0x7f2bc37732b0, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7ffdbef659b0) at gsignal.c:3635
    #12 0x00007f2c0225efda in g_signal_emit_valist (instance=instance@entry=0x7f2bc37732b0, signal_id=signal_id@entry=99, detail=detail@entry=0, var_args=var_args@entry=0x7ffdbef65be8) at gsignal.c:3391
#14 0x00007f2c05099543 in gtk_selection_retrieval_report (info=info@entry=0x7f2bb45c47f0, type=<optimized out>, format=<optimized out>, buffer=<optimized out>, length=<optimized out>, time=time@entry=0) at gtkselection.c:3047
#15 0x00007f2c0509b373 in gtk_selection_convert (widget=0x7f2bc37732b0 [GtkInvisible], selection=0x45, target=0x59, time_=0) at gtkselection.c:1159
#16 0x00007f2bf856d951 in nsRetrievalContextWayland::GetClipboardData(char const*, int, unsigned int*) (this=0x7f2bddf3b430, aMimeType=<optimized out>, aWhichClipboard=<optimized out>, aContentLength=0x7ffdbef65e94) at /home/evan/code/mozilla-central/obj-x86_64-pc-linux-gnu/dist/include/mozilla/mozalloc.h:156
#17 0x00007f2bf8573b55 in nsClipboard::GetData(nsITransferable*, int) (this=0x7f2bddf053d0, aTransferable=0x7f2bac8f9980, aWhichClipboard=1) at /home/evan/code/mozilla-central/widget/gtk/nsClipboard.cpp:333
#18 0x00007f2bf831ba2b in mozilla::dom::ContentParent::RecvGetClipboard(nsTArray<nsTString<char> >&&, int const&, mozilla::dom::IPCDataTransfer*) (this=0x7f2bb4625800, aTypes=nsTArray<nsTString<char> > && = {...}, aWhichClipboard=@0x7ffdbef660c8: 1, aDataTransfer=0x7ffdbef660e8)
    at /home/evan/code/mozilla-central/dom/ipc/ContentParent.cpp:2636
#19 0x00007f2bf6ddf219 in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&, IPC::Message*&) (this=0x7f2bb4625800, msg__=..., reply__=@0x7ffdbef662b0: 0x0) at /home/evan/code/mozilla-central/obj-x86_64-pc-linux-gnu/ipc/ipdl/PContentParent.cpp:8513
#20 0x00007f2bf6d65ca0 in mozilla::ipc::MessageChannel::DispatchSyncMessage(IPC::Message const&, IPC::Message*&) (this=this@entry=0x7f2bb4616900, aMsg=..., aReply=@0x7ffdbef662b0: 0x0) at /home/evan/code/mozilla-central/ipc/glue/MessageChannel.cpp:2109
#21 0x00007f2bf6d6f414 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) (this=this@entry=0x7f2bb4616900, aMsg=...) at /home/evan/code/mozilla-central/ipc/glue/MessageChannel.cpp:2067
#22 0x00007f2bf6d70526 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) (this=0x7f2bb4616900, aTask=...) at /home/evan/code/mozilla-central/ipc/glue/MessageChannel.cpp:1917
#23 0x00007f2bf6d70596 in mozilla::ipc::MessageChannel::MessageTask::Run() (this=0x7f2bc373e020) at /home/evan/code/mozilla-central/ipc/glue/MessageChannel.cpp:1950
#24 0x00007f2bf6a6f1fe in nsThread::ProcessNextEvent(bool, bool*) (this=0x7f2be3e11530, aMayWait=<optimized out>, aResult=0x7ffdbef669af) at /home/evan/code/mozilla-central/xpcom/threads/nsThread.cpp:1090
#25 0x00007f2bf6a70b01 in NS_ProcessNextEvent(nsIThread*, bool) (aThread=aThread@entry=0x7f2be3e11530, aMayWait=aMayWait@entry=true) at /home/evan/code/mozilla-central/xpcom/threads/nsThreadUtils.cpp:519
#26 0x00007f2bf6d66b5f in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) (this=0x7f2be3e24dc0, aDelegate=0x7f2c07261580) at /home/evan/code/mozilla-central/ipc/glue/MessagePump.cpp:125
#27 0x00007f2bf6d3b949 in MessageLoop::RunInternal() (this=this@entry=0x7f2c07261580) at /home/evan/code/mozilla-central/ipc/chromium/src/base/message_loop.cc:326
#28 0x00007f2bf6d3bac1 in MessageLoop::RunHandler() (this=0x7f2c07261580) at /home/evan/code/mozilla-central/ipc/chromium/src/base/message_loop.cc:319
#29 0x00007f2bf6d3bac1 in MessageLoop::Run() (this=0x7f2c07261580) at /home/evan/code/mozilla-central/ipc/chromium/src/base/message_loop.cc:299
#30 0x00007f2bf8538a1b in nsBaseAppShell::Run() (this=0x7f2be1252510) at /home/evan/code/mozilla-central/widget/nsBaseAppShell.cpp:157
#31 0x00007f2bf91b9718 in nsAppStartup::Run() (this=0x7f2be12d0100) at /home/evan/code/mozilla-central/toolkit/components/startup/nsAppStartup.cpp:290
#32 0x00007f2bf924c69c in XREMain::XRE_mainRun() (this=this@entry=0x7ffdbef66d30) at /home/evan/code/mozilla-central/toolkit/xre/nsAppRunner.cpp:4829
#33 0x00007f2bf924ce6f in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) (this=this@entry=0x7ffdbef66d30, argc=argc@entry=4, argv=argv@entry=0x7ffdbef68038, aConfig=...) at /home/evan/code/mozilla-central/toolkit/xre/nsAppRunner.cpp:4974
#34 0x00007f2bf924d1b7 in XRE_main(int, char**, mozilla::BootstrapConfig const&) (argc=4, argv=0x7ffdbef68038, aConfig=...) at /home/evan/code/mozilla-central/toolkit/xre/nsAppRunner.cpp:5066
#35 0x00005622b6d4a428 in do_main(int, char**, char**) (argc=4, argv=0x7ffdbef68038, envp=<optimized out>) at /home/evan/code/mozilla-central/browser/app/nsBrowserApp.cpp:231
#36 0x00005622b6d49d64 in main(int, char**, char**) (argc=4, argv=0x7ffdbef68038, envp=0x7ffdbef68060) at /home/evan/code/mozilla-central/browser/app/nsBrowserApp.cpp:304
(Assignee)

Comment 7

10 months ago
Posted patch wayland-strncpy.patch (obsolete) — Splinter Review
By the time nsClipboard.cpp calls GetClipboardText(), it has lost the information about the size of the underlying string. Therefore mClipboardData must be null-terminated. If it's not null-terminated, the utf-8 append code in nsReadableUtils.cpp will (usually) read past the end of the buffer and think the buffer is invalid UTF-8 data.

The fix is to allocate one extra byte of data, and copy the string contents using strncpy (the GTK selection data itself is already null-terminated).
Attachment #8975307 - Flags: review?(stransky)
(Reporter)

Comment 8

10 months ago
This backtrace is related to Bug 1460810, see wrong __len and no data at __src in the backtrace:

#6  0x00007f2bf856de76 in memcpy (__len=4294967295, __src=0x0, __dest=0x7f2a8b400000) at /usr/include/bits/string_fortified.h:34
(Reporter)

Comment 9

10 months ago
Comment on attachment 8975307 [details] [diff] [review]
wayland-strncpy.patch

Strncpy() does not add extra '\0' at the string end when the '\0' is missing at string source. Better to leave memcpy and just add it explicitly as

mClipboardData[mClipboardDataLength] = '\0';
Attachment #8975307 - Flags: review?(stransky) → review-
(Assignee)

Comment 10

10 months ago
Agree -- after thinking about this more, I realized that strncpy might also be wrong when using copy/paste for images or other binary formats, as those could include null bytes. Will update.
(Assignee)

Comment 11

10 months ago
As before, but instead of using strncpy(), keep the memcpy() and explicitly add a null at the end of the buffer.
Attachment #8975307 - Attachment is obsolete: true
Attachment #8975639 - Flags: review?(stransky)
(Assignee)

Comment 12

10 months ago
Posted patch explicit-length.patch (obsolete) — Splinter Review
Here's another way of fixing this issue. This changes the GetClipboardText() method to return the clipboard length explicitly, which seems safer to me anyway.
Attachment #8975649 - Flags: review?(stransky)
(Reporter)

Comment 13

10 months ago
Comment on attachment 8975649 [details] [diff] [review]
explicit-length.patch

I don't think we need this as as the GetClipboardText() returns null terminated string and the clipboardDataLength is redundant here. GetClipboardText() matches gtk_clipboard_wait_for_text() on Gtk+ side.

Also GetClipboardText() on X11 will return string size + 1 as the size includes the termination '\0' here.
Attachment #8975649 - Flags: review?(stransky) → review-
(Reporter)

Comment 14

10 months ago
Comment on attachment 8975639 [details] [diff] [review]
wayland-null-termination.patch

Thanks, this is what we want.

For further patches please use MozReview to submit a patch for review:

https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch
https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
Attachment #8975639 - Flags: review?(stransky) → review+
(Reporter)

Updated

10 months ago
Assignee: nobody → evan
(Reporter)

Comment 15

10 months ago
Patch for check-in
(Reporter)

Updated

10 months ago
Keywords: checkin-needed
(Reporter)

Updated

10 months ago
Attachment #8975649 - Attachment is obsolete: true
Attachment #8975639 - Attachment is obsolete: true

Comment 16

10 months ago
Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c1d015760f2
[Wayland/Clipboard] Null terminate text string returned by GetClipboardText(), r=stransky
Keywords: checkin-needed

Comment 17

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5c1d015760f2
Status: NEW → RESOLVED
Last Resolved: 10 months ago
status-firefox62: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
status-firefox60: affected → wontfix
status-firefox61: --- → fix-optional
status-firefox-esr60: --- → fix-optional
Is this something that can ride the trains or should it be considered for backport?
Flags: needinfo?(stransky)
(Reporter)

Comment 19

10 months ago
(In reply to Ryan VanderMeulen [:RyanVM] from comment #18)
> Is this something that can ride the trains or should it be considered for
> backport?

No need to backport, 62 is fine here as 61 is missing other related patches like D&D and so.
Flags: needinfo?(stransky)
status-firefox61: fix-optional → wontfix
status-firefox-esr60: fix-optional → wontfix
You need to log in before you can comment on or make changes to this bug.