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

Auto copy-paste with navigator.clipboard API #1301

Closed
wants to merge 4 commits into from

Conversation

akamos
Copy link

@akamos akamos commented Sep 25, 2019

Hi!
I added the automatical copy-paste function that is available through the navigator.clipboard API in some browsers (e.g. Google Chrome) as mentioned earlier in PR #1174.

Co-authored-by: bulldozier [email protected]

Copy link
Member

@samhed samhed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have yet to test this, but if this is something Chrome specific, we could do with some comments in the code.

app/ui.js Outdated Show resolved Hide resolved
app/ui.js Outdated Show resolved Hide resolved
@samhed samhed added the feature label Sep 28, 2019
app/ui.js Show resolved Hide resolved
@samhed
Copy link
Member

samhed commented Oct 28, 2019

I took some time today for testing this. I have tested both Chrome 66 and Chrome 78 on Linux, and Chrome 77 on Windows 10. Things didn't work in any of the browsers, navigator.clipboard was undefined in all cases. Am I missing something?

@akamos
Copy link
Author

akamos commented Nov 4, 2019

I don't know how you run the tests, but clipboard is for me defined in normal desktop environment of Chrome 78 on Windows 10 and Chrome 75 on Windows 7.

EDIT: It works only with HTTPS.

@samhed
Copy link
Member

samhed commented Nov 4, 2019

EDIT: It works only with HTTPS.

Ah, that's probably it. I'll retest with HTTPS when I have the time

@ericfranz
Copy link

Both navigator.clipboard and navigator.clipboard.writeText are defined in Firefox. But Firefox does not support this, so every interaction in NoVNC that invokes writeLocalClipboard will result in the Log.Error being sent to the console. If you do something like below, you can avoid polluting the console when querying the clipboard-write permission would result in a type error:

     writeLocalClipboard(text) {
-        if (typeof navigator.clipboard !== "undefined" && typeof navigator.clipboard.writeText !== "undefined") {
-            navigator.clipboard.writeText(text).then(() => {
+        if (typeof navigator.clipboard !== "undefined" && typeof navigator.clipboard.writeText !== "undefined"
+            && typeof navigator.permissions !== "undefined" && typeof navigator.permissions.query !== "undefined"
+           ) {
+            navigator.permissions.query({name: 'clipboard-write'})
+            .then(() => { return navigator.clipboard.writeText(text) })
+            .then(() => {
                 let debugMessage = text.substr(0, 40) + "...";
                 Log.Debug('>> UI.setClipboardText: navigator.clipboard.writeText with ' + debugMessage);
-            }).catch(() => {
-                Log.Error(">> UI.setClipboardText: Failed to write system clipboard (trying to copy from NoVNC clipboard)");
+            }).catch((err) => {
+                if(err.name !== 'TypeError')
+                    Log.Error(">> UI.setClipboardText: Failed to write system clipboard (trying to copy from NoVNC clipboard)");
             });
         }
     },

ericfranz added a commit to OSC/ondemand that referenced this pull request Dec 5, 2019
ericfranz added a commit to OSC/ondemand that referenced this pull request Dec 10, 2019
NoVNC mod for streamlined copy/paste in chrome novnc/noVNC#1301

If clipboard-read permission results in TypeError we just ignore it
that way if in the future Firefox or other browser defines clipboard.readText but
does not support the read permission we don't get a bunch of console warnings
@ericfranz
Copy link

Another problem with this approach that needs to be addressed is that it makes a preexisting issue worse. Copy to the system clipboard >10232 bytes and paste into the clipboard input field and you will get a RangeError: Invalid typed array length, because that would exceed the queue buffer size for a message (the text + 8 chars for buffer):

this._sQbufferSize = 1024 * 10; // 10 KiB

In Chrome with this PR the problem is made worse due to the automation because if you just copy to the system clipboard >10232 bytes the NoVNC browser tab will immediately display the RangeError. Reload the page and you will get the RangeError again.

samirmansour pushed a commit to OSC/ondemand that referenced this pull request Feb 14, 2020
NoVNC mod for streamlined copy/paste in chrome novnc/noVNC#1301

If clipboard-read permission results in TypeError we just ignore it
that way if in the future Firefox or other browser defines clipboard.readText but
does not support the read permission we don't get a bunch of console warnings
ericfranz pushed a commit to OSC/ondemand that referenced this pull request Jun 23, 2020
Applies novnc/noVNC#1301 solution for copy and paste.
@samhed
Copy link
Member

samhed commented Jul 1, 2020

Let's continue this on #1347

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.

3 participants