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

Don't throw an error if access to the clipboard is denied. #6516

Merged
merged 1 commit into from Nov 11, 2019

Conversation

azatsarynnyy
Copy link
Member

@azatsarynnyy azatsarynnyy commented Nov 8, 2019

Signed-off-by: Artem Zatsarynnyi azatsary@redhat.com

What it does

When the user has denied access to the clipboard, it should not be treated as an error, by Theia.
The user just needs to be warned about it.

fixes eclipse-che/che#15093

How to test

1/ Run Theia with the test extension https://github.com/azatsarynnyy/test-clipboard-api/blob/master/clipboard-api-test-0.0.1.vsix
2/ Deny the browser accessing the clipboard from Theia page.
3/ Call Clipboard: read from clipboard command from the commands palette.

w/o the patch
image

w/ the patch
image

Review checklist

Reminder for reviewers

packages/core/src/browser/browser-clipboard-service.ts Outdated Show resolved Hide resolved
}
}
if (permission.state === 'denied') {
// most likely, the user intentionally denied the access
this.messageService.error("Access to the clipboard is denied. Check your browser's permission.");
throw new Error('Access to the clipboard is denied.');
this.messageService.warn("Access to the clipboard is denied. Check your browser's permission.");
Copy link
Member

Choose a reason for hiding this comment

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

Is not it confusing that we use the same warning message for read and write? Would not it better to have Read access and Write access instead of just Access?

Copy link
Member Author

Choose a reason for hiding this comment

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

But there're no separate read/write settings available to the user. It's just general access.

Copy link
Member

Choose a reason for hiding this comment

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

ok, I was concerned to know from where an error is coming, right now it is not clear whether extension tried to read or write. probably it is not important, let's wait with it.

@akosyakov akosyakov added clipboard issues related to the clipboard vscode issues related to VSCode compatibility labels Nov 8, 2019
@azatsarynnyy
Copy link
Member Author

@akosyakov I've addressed your comments. Thanks for reviewing!

Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

looks good, please clean up the history (squash commits) and merge

Signed-off-by: Artem Zatsarynnyi <azatsary@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clipboard issues related to the clipboard vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[openshift plugin] improve cluster login flow when Che is on non-secure connection
2 participants