-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Clipboard API for Plugins #5994
Conversation
@azatsarynnyy is it ready for a review? |
@akosyakov it's not ready yet. I want to create another test plugin which will test Clipboard API only. I'll ping you once it's ready. |
It's ready for review and test |
I'm offline tomorrow. If someone gives a try proposed test cases in the description on different browsers it would be helpful. Also if something does not work think about suggestion to fix them, i.e. use DOM api as we do in some places in the code for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works well on Chrome! However, it did not seem to work on Firefox. Running the commands did not have any effect, and there was no browser pop-up requesting permission.
It is strange. Latest Firefox supports should support these APIs. @fangnx which version of Firefox do you use? @azatsarynnyy could you debug please what is going wrong on FireFox. Theia claims to support latest version of Firefox and Chrome. |
We need to verify as well that @benoitf use case is working: #5527 (review) |
Firefox Quantum 68.0.1 (64-bit), Ubuntu 18.04.1 |
From https://developer.mozilla.org/en-US/docs/Web/API/Clipboard#Browser_compatibility:
So basically FireFox does not support these APIs yet :( We need a fallback strategy. |
Also found in https://developer.mozilla.org/en-US/docs/Web/API/Clipboard/read:
So maybe we should no |
Thank you for testing it, guys! 🙇♂️ |
It doesn't work in FF for me also. I'm going to investigate the problem. |
Async Clipboard API works well in Chrome without asking the permissions. |
@azatsarynnyy Can we somehow detect it and notify a user to enable it? Is there any security concerns with enabling it? |
I think relying on Although FF declares support of Async Clipboard API, it's exposed to secure contexts and browser extension only. So, looks like we cannot use it for this task.
As I understand, the only way we can follow is by relying on some 3rd party lib. |
I don't think that 3rd party lib will do anything special, just calling
Could we try it anyway, like suggested in #5994 (comment)? We are already using such approach in some places it was good so far. It should be only a fallback. |
@akosyakov the reason why we cannot use The main issue here is that FF requires calling I examined all the places in Theia where I tried to call async writeText(value: string): Promise<void> {
const onCopy = (e: ClipboardEvent) => {
document.removeEventListener('copy', onCopy);
if (e.clipboardData) {
e.clipboardData.setData('text/plain', value);
e.preventDefault();
}
};
document.addEventListener('copy', onCopy);
document.execCommand('copy');
} It worked well in both, Chrome and FF. But when I call it through the Plugin API, it works in Chrome only. And FireFox hints in the console with the following message: I believe it's because of |
To allow the user using Clipboard Plugin API in FireFox, I don't see any other options except recommending to enable |
@azatsarynnyy Let's notify a user with an error that he head to enable it and how in FF, i.e. point to a some FF page which explains it (with security concerns). Do I understand correctly that then |
Thought a bit about it again, we still want to have a fallback for native extensions, since they can call |
correct |
I've updated the PR with FF support. Also, the PR description has been updated. |
Signed-off-by: Artem Zatsarynnyi <[email protected]>
bafbe6d
to
2fd659b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works very well for me.
@azatsarynnyy please file a follow-up issue to use |
@azatsarynnyy please merge it |
The follow-up issue #6209 |
What it does
Adds Clipboard API for Plugins.
Closes #5519
This PR is based on the code extracted from #5527
Some notes about the implementation
Different browsers and it's native extensions work a bit differently with the APIs around the clipboard: Async Clipboard API, Permissions API, document.execCommand API.
So the PR adds a minimal level of support for Chrome and FireFox browsers. Support for other cases may be added later as needed.
How to test
Run Theia with the test VS Code extension https://github.com/azatsarynnyy/test-clipboard-api/blob/master/clipboard-api-test-0.0.1.vsix
To test
vscode.env.clipboard.readText
API:Clipboard: read from clipboard
commandTo test
vscode.env.clipboard.writeText
API:Clipboard: write selection to clipboard
commandBrowser-specific behavior:
The user is able to block the access to the clipboard by setting a preference
In that case, Theia shows the message
By default, Async Clipboard API is disabled but it can be enabled by setting
dom.events.testing.asyncClipboard
preference. User will be informed about that with a notificationBesides that, Async Clipboard API isn't gated with the permissions in FireFox, so it just bypassed.
Review checklist
Reminder for reviewers