Fix using copy/paste from a menu in electron. #13220
Conversation
Contributed on behalf of STMicroelectronics Signed-off-by: Thomas Mäder <t.s.maeder@gmail.com>
There was a problem hiding this comment.
Changes look good to me and solve the original problem, thank you Thomas! From what I understand the isNative flag was really just introduced for the copy/paste scenario to distinguish between the Electron and the Browser case (see here) but who can say for sure ;-) Anyway, great work and I just left two comments regarding the browser API that we can probably skip for now.
| // Chrome incorrectly returns true for document.queryCommandSupported('paste') | ||
| // when the paste feature is available but the calling script has insufficient | ||
| // privileges to actually perform the action | ||
| export const supportPaste = browser.isNative || (!browser.isChrome && document.queryCommandSupported('paste')); | ||
| export const supportPaste = environment.electron.is() || (!browser.isChrome && document.queryCommandSupported('paste')); |
There was a problem hiding this comment.
I'm not sure that we need the special Chrome handling anymore, see w3c/editing#167, https://bugs.chromium.org/p/chromium/issues/detail?id=757140#c4, and https://chromium.googlesource.com/chromium/src.git/+/29e7204c4d91b754fe8df8e264695a695a5ab974.
| @@ -351,12 +351,12 @@ export namespace CommonCommands { | |||
| }); | |||
| } | |||
|
|
|||
| export const supportCut = browser.isNative || document.queryCommandSupported('cut'); | |||
| export const supportCopy = browser.isNative || document.queryCommandSupported('copy'); | |||
| export const supportCut = environment.electron.is() || document.queryCommandSupported('cut'); | |||
There was a problem hiding this comment.
Just to note it down somewhere cause I quickly looked into it: queryCommandSupported is officially deprecated but still supported in all major browsers. From what I gather, we could use the Clipboard API as an alternative.
|
Using the browser copy/paste commands is not really what we want: we want to react differently depending on the source/target of the paste: for example, "paste" will always paste into the widget with keyboard focus even though the active widget might be the Navigator, for example. There is a related issue talking about the Terminal, for example: #10724 |
What it does
The fix is to replace a old test for whether we run on electron with on that works on current electron.
Fixes #12487
Contributed on behalf of STMicroelectronics
How to test
Start Theia Electron and make sure that copy/pasting from a menu actually works (for example in the terminal or in the editor)
Follow-ups
Review checklist
Reminder for reviewers