Add clipboard support, continued

[snickell]

This PR is a continuation of @juanjoDiaz 's hard work here: #1347

At the moment, its identical to #1347 , but with the merge conflict relative to noVNC:master resolved. I'll continue to push to this PR as required to resolve issues and get this ready for @samhed 's approval.

Please close this PR if its not helpful, but I don't think the other PR can be continued without @juanjoDiaz being involved, so it seemed like a new PR was necessary to fix #1511

@juanjoDiaz explained it better than I can:

This PR adds support for automatic copy-pasting. So when you are focused on the canvas and paste text it's pasted in the server and when you copy something in the server it's automatically copied to your local keyboard.

I'll try to keep a task list updated here for known remaining tasks:

• [x] npm test passes on Chrome
• [x] npm test passes on Safari
• [x] npm test passes on Firefox
• [ ] Investigate and document best-possible approach to a good-ux fallback on Firefox (assuming that's necessary)
• [ ] Validate current implementation on Chrome
• [ ] Validate current implementation on Safari
• [ ] Document behavior of current implementation on Firefox
• [ ] Document behavior of current implementation on Edge
• [ ] Document behavior of current implementation on iOS
• [ ] Document behavior of current implementation on Android

[snickell]

After the merge conflict was resolved + added an expect(Clipboard.isSupported) to the tests, to assert we want the clipboard to be a working thing on all our target/test platforms.

all npm test are passing on my system with Chrome:

SUMMARY:
✔ 504 tests completed
ℹ 6 tests skipped

[snickell]

@samhed / @CendioOssman : anything else that should be on the current task list for moving this toward mergeable?

[snickell]

npm test results, including @juanjoDiaz 's test.clipboard.js tests:

Chrome: all tests pass

Firefox: all tests pass (!)

Safari 14.0.3 (=old version, will retest on current Safari later): automatic clipboard tests fail with exceptions on:

The offending line appears to be the new DataTransfer() constructor not being implemented, which

1. is only referenced in the tests
2. appears to have be implemented in the current version of Safari.

If this works on the current version we should be fine, because this needs-current-safari feature is only required to run the clipboard-test.js tests (e.g. to synthetically inject paste events), NOT to use the actual clipboard.js implementation as a user/client

SUMMARY:
✔ 502 tests completed
ℹ 6 tests skipped
✖ 3 tests failed

FAILED TESTS:
  Automatic Clipboard Sync
    ✖ incoming clipboard data from the server is copied to the local clipboard
      Safari 14.0.3 (Mac OS 10.15.6)
    Illegal constructor
    [native code]
    tests/test.clipboard.js:36:51

    ✖ should copy local pasted data to the server clipboard
      Safari 14.0.3 (Mac OS 10.15.6)
    Illegal constructor
    [native code]
    tests/test.clipboard.js:55:51

    Protocol Message Processing After Completing Initialization
      Normal Clipboard Handling Receive
        ✖ should fire the clipboard callback with the retrieved text on ServerCutText
          Safari 14.0.3 (Mac OS 10.15.6)
        Illegal constructor
        [native code]
        _handleServerCutText@core/rfb.js:1875:55
        _handleMessage@core/rfb.js:869:41
        _handleMessage@[native code]
        _recvMessage@core/websock.js:342:40
        _recvMessage@[native code]
        _receiveData@tests/fake.websocket.js:62:27
        tests/test.rfb.js:2169:53

[snickell]

Bingo, even the test code now passes on current version of Safari (14.1.1):

SUMMARY:
✔ 505 tests completed
ℹ 6 tests skipped
12 07 2021 18:02:04.380:WARN [Safari 14.1.1 (Mac OS 10.15.7)]: Disconnected (0 times) Client disconnected from CONNECTED state (server shutting down)

So in conclusion, I believe the auto-clipboard /does/ work on even old or current versions of Safari (by testing), but the auto sync clipboard tests fail unless you use a recent Safari version, which seems like no big deal to me?

Safari: tests pass Firefox: tests pass Chrome: tests pass

Next I'll try interactive usage to validate the feature, we'll see if Firefox actually now magically works or if the tests need updating to properly break!!!

[juanjoDiaz]

I'd be happy to fix the conflicts and complete any needed work on my original PR as long as the maintainers are willing to merge it.

A big portion of my efforts to improve noVNC have been either rejected or introduced by the maintainers bypassing my PRs. So I am not confortable putting more effort here unless it's clear that the maintainers are willing to merge the PR. If that's the case, @samhed please let me know and I can rebase my PR and amend anything that needs to be amended.

[snickell]

@juanjoDiaz that would be amazing! I totally understand the feeling, I've been on both sides of this fence before..... please let me know if there's anything I can do to help out, sometimes a small tag team at the end can help, if only because it helps maintainers like @samhed know that somebody else has validated a PR (e.g. cross-platform) if they don't have the time/energy/other to check it themselves

[jk4235]

@juanjoDiaz @snickell that is really nice work thanks! hope this PR would be merged soon

[samhed]

Thanks for moving this forward @snickell, I'm back from my vacation now. Since you're on top of this at the moment, let's continue here in this PR. Have you had a chance to look at Firefox?

@juanjoDiaz I believe we have given proper attribution when commiting your changes in the past.

[jabbera]

Am I missing something here? I merged these changes into 1.2 and while copying out to the clipboard works, pasting something copied from outside the browser pastes what was originally copied from the session, not what was copied outside the browser.

[samhed]

You still around @snickell?

[jabbera]

It turns out tightvnc doesn't support copy from virtual session into system clipboard. I'm looking at replacing it.

[crispy-peppers]

Any update on this? Also does this issue looks similar? https://github.com/fcwu/docker-ubuntu-vnc-desktop/issues/272

[MaximMonin]

https://github.com/ceresimaging/noVNC/pull/1

my way to add paste latin clipboard text to TTY (or graphical field) by pressing ctrl-v or shift-Ins

[pavelfeldman]

@snickell

This did not quite work for me when I applied it locally. Even when I fixed it to actually dispatch the handleCopy/handlePaste.

Here is the logic I would suggest following:

1. When focus is switched into the viewer, send the clipboard content to the target. That way any app can access it and paste upon corresponding keyboard or mouse event. Note that since this exposes clip content across the machines, it needs explicit setting / opt-in. readText is now universally available.

window.addEventListener('focus', () => {
  navigator.clipboard.readText().then(text => this.clipboardPasteFrom(text));
});

2. When Copy is invoked, copy the contents of the Pastebin into the clipboard. It already contains the good value. This requires no permissions.

document.addEventListener('copy', () => {
  navigator.clipboard.writeText(document.getElementById('noVNC_clipboard_text').value);
});

That's pretty much it. It also requires to not preventDefault / stopEvent(e) on the Control, otherwise Ctrl+C won't dispatch the copy event and we don't want to simulate it for accessibility.

Would you be interested in getting a patch with those changes?

[cebas]

@pavelfeldman

Would you be interested in getting a patch with those changes? Yes, I am. Thanks

[samhed]

It just gets confusing to have multiple PR's open on the same topic, #1347 has slightly more recent activity, closing this one.

To confirm — we are still interested in merging this type of feature. The problem with this pull request is that we haven't had any activity from the author since July 2021.