Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add automatic clipboard support #1347

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

juanjoDiaz
Copy link
Contributor

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've seen #1301 and #1174. But this takes a very different approach since it adds support for copy-pasting to noVNC's core. And I think that it's quite cleaner.

I've tested it in Safari, Firefox, and Chrome.
Unfortunately, the Clipboard API is only supported by Chrome and the paste event is broken (see https://bugs.chromium.org/p/chromium/issues/detail?id=634426).
So it just does nothing in Safari and Firefox, while in Chrome the copying of data works but not the pasting.

So, for now, this is not super useful but it should become better as browser support improves.

WDYT?

@juanjoDiaz juanjoDiaz force-pushed the add_clipboard_support branch 7 times, most recently from 49ae1a9 to e6b643c Compare December 19, 2019 13:30
@CendioOssman
Copy link
Member

Looks nice and clean, so a good start. But if browser support is too poor then I don't think we want to merge it yet.

I'm not sure how paste is supposed to work even with that chrome bug fixed. We intercept Ctrl+V, so what will be firing the paste event?

@juanjoDiaz
Copy link
Contributor Author

This works in 2 ways:

  1. The Clipboard class captures the ClipboardEvent on the canvas of type paste and paste the text content to the server using RFB's clipboardPasteFrom method.
  2. The cut/copy events from the server that are handled by the RFB's _handle_server_cut_text method now also trigger a ClipboardEvent on the canvas of type copy. The Clipboard class captures the event and updates the local clipboard.

It's true that support varies. However, all browsers seem to be working on it and it's a change that doesn't break anything so I don't really see a reason to hold back.

@samhed samhed added the feature label Jan 8, 2020
@samhed
Copy link
Member

samhed commented Jan 8, 2020

According to MDN this is only supported on Chrome and Opera. Not Firefox, IE, Edge or Safari. I think that this can result in a lot of confused users.

Where can I find information about all browsers working on this?

@samhed
Copy link
Member

samhed commented May 1, 2020

ping @juanjoDiaz

@juanjoDiaz
Copy link
Contributor Author

Sorry @samhed, totally missed this.
At the moment I've moved to work on very different stuff so I'm not actively looking into this.

You are right. It seems that:

One thing to keep in mind is that this PR was mean to be just the foundation of a possible clipboard functionality and it just uses the Clipboard Async API if available.
It could be extended to fallback to the Sync API if available or to use browser-specific workarounds (like https://stackoverflow.com/questions/40147676/javascript-copy-to-clipboard-on-safari) if possible.
We could look into a library like https://github.com/zenorocha/clipboard.js (just to see the approaches that they use, since you guys don't like external dependencies) to maximize browser compatibility.
This page also seems to be useful for browser testing https://whatwebcando.today/clipboard.html

@juanjoDiaz juanjoDiaz force-pushed the add_clipboard_support branch 2 times, most recently from a9c29d8 to 8a38fdd Compare May 4, 2020 13:58
@samhed samhed removed the noresponse label Jul 1, 2020
@samhed
Copy link
Member

samhed commented Jul 1, 2020

https://github.com/zenorocha/clipboard.js

The list of supported browsers there looks great. If they are able to achieve that so should we be! What's the state of this code at the moment?

@juanjoDiaz
Copy link
Contributor Author

The async clipboard support (which is what's used in this PR) is now also supported in Safari: https://webkit.org/blog/10855/async-clipboard-api/

Firefox keeps having it as an open issue: https://bugzilla.mozilla.org/show_bug.cgi?id=1619251

Just FYI.

@GirlBossRush
Copy link

@juanjoDiaz @samhed,

This PR looks like it would get noVNC much closer to an expected experience -- especially now that Chrome, Edge, and Safari support the async Clipboard API. If browser support is a blocker, I could follow up with a new PR that:

  • Add an browser API check when defining the clipboard handler
  • Log a console warning if either copy/paste event is unsuccessful
  • Update the docs and example to include info about noVNC's clipboard API and its requirements

Thoughts?

@samhed
Copy link
Member

samhed commented Sep 21, 2020

I'm opposed to merging something that isn't supported by all major browsers, Firefox is important.

I don't have the time to investigate this personally right now, but https://github.com/zenorocha/clipboard.js claims support for Firefox as well, couldn't we do what they do?

@GirlBossRush
Copy link

GirlBossRush commented Sep 21, 2020

@samhed from what I understand, Firefox's limitations are mostly around async clipboard events e.g. when noVNC wants to copy data to the user's clipboard without their input.

Clipboard.js depends on a "user originating" event to perform a clipboard copy. I suppose a stop-gap would be to offer a button in the UI to let the user retrieve the server's clipboard data.

@AdrianSanchezLopez
Copy link

Hi guys, this feature looks really neat, I was wondering if you have any ETA on this

@snickell
Copy link

snickell commented Jul 7, 2021

@CendioOssman this is something we'd love to have for our novnc install, and I'm happy to do the dev work necessary if there's any requests you have.... any sense what you'd want to see here (besides no active merge conflicts and it actually tested and working) to consider a merge?

@CendioOssman
Copy link
Member

What I'd like to see broad support in the browsers. I.e. this works on at least Chrome, Firefox and Safari (Edge would be nice too though).

Failing that, a clean fallback handling until missing pieces are in place in all browsers.

I haven't kept track of this so I'm afraid I don't know what the current state of the browsers is, or how well this code matches that. Some testing would be helpful to get a better picture.

@cebas
Copy link

cebas commented Mar 21, 2023

All checks have passed. We're closer to have clipboard support! Thanks @juanjoDiaz

@juanjoDiaz
Copy link
Contributor Author

Hi all,

I've rebased and reverted the hacks that I tried for Safari.

This PR has been open now for 3.5 years. Browsers have evolved a bit, but the the problems not so much 😅

Firefox continues to not support reading from clipboard. They seem to be working on it but it will be based on the annoying "Paste" button as Safari is. See https://bugzilla.mozilla.org/show_bug.cgi?id=1770358

Safari continues to work based on the "Paste" button. So, with the current implementation based on the focus event. Whenever you focus on the canvas you get a "Paste" button. If you click on that button, then your data gets copied over. See https://webkit.org/blog/10855/async-clipboard-api/.

So, not much else that I can do here.
I could add a condition so the focus event is not used in Safari and Firefox to avoid the annoying "Paste" button. But that's about it.

@samhed
Copy link
Member

samhed commented Mar 24, 2023

Thanks for moving this forward!

I tested your new code, but I still can't make it work well on Safari on macOS. I do get the paste button more rarely now, but it still gets “stuck” occasionally. And even when the paste button shows, it doesn't paste things.

What I did:

  1. Opened Safari and Notes next to each other on macOS
  2. Connected to a TigerVNC server (linux host) using noVNC
  3. Wrote some text in Notes and Copied it using ⌘-c
  4. Verified successful copy by pasting in Notes using ⌘-v
  5. Clicked in noVNC in Safari
  6. Tried pasting to the remote using Ctrl-v, Ctrl-Shift-v, ⌘-v and middle-mouse => Nothing
  7. Clicked outside the noVNC remote screen, but still in Safari, then clicked back in noVNC remote
  8. I got a paste button that wouldn't go away, when I clicked it, it went away for half a second then reappeared
  9. After spamming different buttons on the mouse for a while, it finally gave up showing the paste button (after ~15 seconds perhaps?)
  10. When then I pasted using the middle-mouse to the remote, I get the copied text from Notes

I cleared all browser caches in Safari and made sure I pulled the latest version from your branch. Are we certain that the Safari issue with the paste button is properly fixed?

As described above, pasting appears to be possible in Safari on macOS (but still buggy). But copying from the remote to the macOS client doesn't work whatsoever for me. Have you gotten that to work on your end?

I also tested Safari on iPadOS, it still doesn't seem to work at all sadly. As I wrote in an earlier post, it seems it should be possible:

When testing this fiddle in Safari on an iPad things work well, both Copy and Paste:
https://jsfiddle.net/developit/c0aw4sua/

I also tested in both Chrome and Firefox on Fedora. Chrome works flawlessly, it's really nice! Firefox works really well regarding copy, just like you have said.

To summarize, I get the feeling this needs further polishing for Safari, both on macOS and iPadOS/iOS. If we get that working well, I believe we can merge this. We would have a good foundation ready for Firefox once their paste-feature gets released.

@juanjoDiaz
Copy link
Contributor Author

Clicked outside the noVNC remote screen, but still in Safari, then clicked back in noVNC remote
I got a paste button that wouldn't go away, when I clicked it, it went away for half a second then reappeared
After spamming different buttons on the mouse for a while, it finally gave up showing the paste button (after ~15 seconds perhaps?)

This is the "onFocus" workaround.
We sync the clipboard during the focus event.
On Safari, that triggers the "paste" button.

This is why suggested disabling the pasting completely in Safari.

I also tested Safari on iPadOS, it still doesn't seem to work at all sadly. As I wrote in an earlier post, it seems it should be possible:

When testing this fiddle in Safari on an iPad things work well, both Copy and Paste:
https://jsfiddle.net/developit/c0aw4sua/

No, it doesn't.
It works if you copy using the "copy" button and paste using the "paste" button.
If you copy text for somewhere else and press the "paste" button, you get the annoying "paste" floating menu.
image

So, all in all, Chrome works so nicely because of the "focus" workaround.
Safari (and in the future Firefox seems like will follow the same approach) prevent this type of workarounds by showing the "paste" floating menu.

So the real question is: what do we do with pasting on Safari and Firefox?
They are trying to actively limit it so it only works if the user press a "paste" button and then the "paste" floating menu; i.e. we can only achieve this by adding a "paste" button on the noVNC UI. But it won't work as "natively" as Chrome does.

Wdyt?

@samhed
Copy link
Member

samhed commented Mar 27, 2023

Sorry, I wasn't clear. I have no problem at all with the concept of a floating “paste” button. I think it is an acceptable inconvenience when weighed against the safety issues Chrome users have to face.

When testing this fiddle in Safari on an iPad things work well, both Copy and Paste:
https://jsfiddle.net/developit/c0aw4sua/

No, it doesn't.
It works if you copy using the "copy" button and paste using the "paste" button.
If you copy text for somewhere else and press the "paste" button, you get the annoying "paste" floating menu.

What I meant is that it is possible to copy/paste stuff on an iPad using that jsfiddle, but not when using noVNC. I'm not even getting the paste button on iPad when using noVNC.

So the real question is: what do we do with pasting on Safari and Firefox?
They are trying to actively limit it so it only works if the user press a "paste" button and then the "paste" floating menu; i.e. we can only achieve this by adding a "paste" button on the noVNC UI. But it won't work as "natively" as Chrome does.

Wdyt?

Yeah, I think we need to embrace the floating paste button. But I would appreciate it if we could avoid another button in the noVNC UI.

I think the focus workaround needs further tweaks to not break things on Safari. The focus handler seems to trigger too much. My guess is that clicking the floating focus button briefly removes focus from the page (I have not verified this).

Do you think it would be possible to find a solution that automatically shows the floating paste button at the cursor only when we want it to?

@juanjoDiaz
Copy link
Contributor Author

Sorry, I wasn't clear. I have no problem at all with the concept of a floating “paste” button. I think it is an acceptable inconvenience when weighed against the safety issues Chrome users have to face.

hehe we are not understanding each other here 😅

The focus handler seems to trigger too much. My guess is that clicking the floating focus button briefly removes focus from the page (I have not verified this).

That's it!
The problem is on how to sync the local and remote clipboard.
Due to security restrictions, all browsers require that you do it as part of a user action.
Which action should we use? Based on the discussion above, I opted for sync the clipboard during the focus event.
That works great in Chrome but means that every focus event on the canvas will show the "Paste" button.
And for some reason, the canvas seems to focus/blur quite often.

Should we use the click event?
That doesn't sound much better. Every user click will show the paste button.

Keypresses don't sound great either...

Which other event do we have that we can use?

So, it seems that adding a button to noVNC UI is the only working approach here... 😕

@samhed
Copy link
Member

samhed commented Mar 28, 2023

Thanks for clarifying!

Perhaps we can continue using "focus".. Can we detect if the browser is using the "paste" button approach? If we can detect it in a nice way, we could apply some sort of rate limiting or other kind of workaround to make it usable. What do you think?

@juanjoDiaz
Copy link
Contributor Author

Hi!

Sorry, I didn't see your answer.

AFAIK, we can't detect if the browser is using the "paste" button other than detecting if the browser is chrome, firefox or safari.

@samhed
Copy link
Member

samhed commented Apr 24, 2023

In case we expect the "paste" button to appear on focus, perhaps we can approximately detect it by seeing if we immediately lose focus again?

The next steps should be to solve these two issues:

  • limit the "paste" button spam
  • ensure the "paste" button is visible even if remote screen covers the entire browser window

@spags-lacework
Copy link

Hi, is there any recent update on the status of this PR?

@I0x0I
Copy link

I0x0I commented Mar 19, 2024

It seems this issue is resolved in a forked version kasmtech/noVNC

@MaximMonin
Copy link

Firefox 127 added more clipboard api support

@pleandre
Copy link

pleandre commented Jun 18, 2024

In Apache Guacamole, they seems to use a mix of the Asynchronous Clipboard API like you do and some hacky way with a textarea as a fallback
https://github.com/apache/guacamole-client/blob/main/guacamole/src/main/frontend/src/app/clipboard/services/clipboardService.js#L192

And they sync clipboard on these events
https://github.com/apache/guacamole-client/blob/main/guacamole/src/main/frontend/src/app/index/controllers/indexController.js#L276

image

Their explanation: https://guacamole.apache.org/faq/#clipboard
image

This is what Apache team ended up with after 6 or 7 PRs and 8 years of evolution.
https://issues.apache.org/jira/browse/GUACAMOLE-55
https://issues.apache.org/jira/browse/GUACAMOLE-310
https://issues.apache.org/jira/browse/GUACAMOLE-128
https://issues.apache.org/jira/browse/GUACAMOLE-559
etc.
Their implementation also supports copy pasting images.

@samhed
Copy link
Member

samhed commented Jun 20, 2024

Sounds promising that Firefox added more support in 127! I would love to see this get merged.

I'd be happy to help out with testing if the PR is updated to address the issues mentioned above:

In case we expect the "paste" button to appear on focus, perhaps we can approximately detect it by seeing if we immediately lose focus again?

The next steps should be to solve these two issues:

  • limit the "paste" button spam
  • ensure the "paste" button is visible even if remote screen covers the entire browser window

@samhed
Copy link
Member

samhed commented Jun 20, 2024

@pleandre thank you for looking into what Apache achieved! If you are up for it, feel free to open a new PR. We'll help out as best we can.

@williamsjoblom
Copy link
Contributor

Below I have tried to summarize the discussion, as well as doing some testing on
my own to hopefully gain some traction in the issue.

  1. [safari] Paste prompt is intrusive

    As per MDN, Safari will show a paste prompt before the browser gets to access
    the current content of the clipboard.

    In Firefox, this prompt is rather non-intrusive as it pops up on the bottom
    of the screen. In Safari on the other hand, it pops up next to the mouse,
    covering the VNC viewport. This in itself is a bit annoying.

    To make matters worse, the current implementation in this PR tries to read
    the clipboard memory every time the viewport canvas gains focus. This, in
    combination with the fact that Safari lets go of the focus when displaying
    the paste prompt, make the paste prompt show up all the time.

    In contrast, when Firefox shows its paste prompt, the canvas does not lose
    focus.

    This problem is very much a blocker for merging this PR as highlighted by
    previous commenters.

    One possible way around this would be to check if the browser window has lost
    and gained focus since the last time the canvas gained focus. I'm not 100%
    certain that this will work, but it may be worth a shot.

  2. [firefox] Paste prompt is easy to miss

    Similar to Safari, Firefox will also show a paste prompt. This one, however,
    is easy to miss as it is displayed at the bottom of the window.

  3. [firefox, safari] "Paste" prompt is confusing in this context

    In both Safari and Firefox (that prompt the user before giving up the current
    state of the clipboard memory), this prompt says "Paste". This is not intuitive
    at the slightest as clicking does not paste anything. All it does is sending
    the current client clipboard to the server.

    This has a high potential to confuse new users.

  4. [general cleanup] Copying from server to client is triggered by the canvas
    copy event. These events can both be fired by the browser itself when it
    detects a client-native copy actions and programmatically when the receiving a
    ServerCutText message. The copy event handler then calls
    clipboard.writeText.

    Based on some testing, I have not been able to get the browser (Firefox 127
    and Chrome 126) to fire these events. All events I have observed has been
    fired from the ServerCutText handler in rfb.js. If the browser never
    fires these events when copying text inside the session, does it really make
    sense to indirectly call Clipboard._handleCopy from
    RFB._handleServerCutText by firing an artificial event?

    This is further strengthened by Apaches reasoning to not rely on the browser
    fired copy events: https://guacamole.apache.org/faq/#clipboard.

    In my opinion, directly calling Clipboard._handleCopy from
    RFB._handleServerCutText would remove this complexity.

To conclude, Chrome currently works without a hiccup and gives a great user
experience. Safari and Firefox (the browser with the largest market share after
Chrome) is more problematic.

My suggestion would be to consider Chrome as a separate entity than Safari and
Firefox. That is, use the work in this pull request exclusively in Chrome, and
treat Safari and Firefox separately.

@williamsjoblom
Copy link
Contributor

williamsjoblom commented Jul 3, 2024

One way we could differentiate between the Chrome way of doing things and the Firefox/Safari way would be to look for the presence of the clipboard-read and clipboard-write permissions. If they are present, the browser in question is likely following Chromes example.

That is, of course, if we are dead set on avoiding looking at the user agent or window.isChrome.

@williamsjoblom
Copy link
Contributor

williamsjoblom commented Jul 4, 2024

Worth noting is that Firefox has this issue still open, indicating that the UX of the paste button may change in the future:

https://bugzilla.mozilla.org/show_bug.cgi?id=1777197

Moreover, the issues in Safari that I highlighted above are also reproducible in Epiphany on Linux.

@0xAungkon
Copy link

Man I Fixed This Automatic Clipboard Snyc Issue XD . Check the latest pull .

@juanjoDiaz @CendioOssman @samhed @GirlBossRush @AdrianSanchezLopez

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.