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

Desktop: Resolves #7880: Paste as Text only working on hotkeys on Windows #7886

Merged
merged 3 commits into from
Mar 12, 2023
Merged

Desktop: Resolves #7880: Paste as Text only working on hotkeys on Windows #7886

merged 3 commits into from
Mar 12, 2023

Conversation

pedr
Copy link
Collaborator

@pedr pedr commented Mar 7, 2023

Resolves #7880

When making the implementation for the Paste as Text functionality I use the clipboard.read function which is still experimental and doesn't seem to work on Windows.

To fix that I just use the simpler function readText

https://www.electronjs.org/docs/latest/api/clipboard

How to Test

  • On Windows
  • Copy content to the clipboard
  • Inside the Rich Text Editor, right-click inside the editor and select 'Paste as Text' or select the option from the top menu Edit -> Paste as Text
  • The copied content should appear on the screen

If the command works with other shortcuts:

  • On Windows
  • Rebind 'Paste as Text' to Ctrl+M
  • Copy content to Clipboard
  • Inside the Rich Text Editor, try to paste content with the shortcut
  • The copied content should appear on the screen

With Google Docs:

With empty content:

  • On Windows, inside the Rich Text Editor
  • Clean clipboard (Windows+V to be able to access clipboard utility)
  • Paste as Text does not appear as an option inside the Context Menu since it has nothing to paste
  • Pasting with the upper menu option should call the command, but nothing should be inserted since there is no content inside the clipboard

@pedr pedr requested a review from laurent22 March 7, 2023 19:23
@laurent22
Copy link
Owner

In the "How to Test" section there should be more than testing the specific function. We should also check that nothing else got broken, for example what if the clipboard only contains HTML but not text data? In that case isn't it going to paste nothing? Also maybe check what happens when pasting from Word (or Wordpad if you don't have Word), LibreOffice or Thunderbird. There have been issues with these in the past so we should check there's no regression

@pedr
Copy link
Collaborator Author

pedr commented Mar 7, 2023

In the "How to Test" section there should be more than testing the specific function. We should also check that nothing else got broken, for example what if the clipboard only contains HTML but not text data? In that case isn't it going to paste nothing? Also maybe check what happens when pasting from Word (or Wordpad if you don't have Word), LibreOffice or Thunderbird. There have been issues with these in the past so we should check there's no regression

I'm not sure how to copy HTML content that is empty so I couldn't test that, but I tested with an empty clipboard, and with Google Docs, since when copying from Wordpad it doesn't bring any styles, and with Gmail content.

@laurent22 laurent22 changed the base branch from dev to release-2.10 March 8, 2023 14:19
@pedr
Copy link
Collaborator Author

pedr commented Mar 9, 2023

Just so you know when this PR or #7885 get merged it will (probably) create a merge conflict.

@laurent22
Copy link
Owner

Just so you know when this PR or #7885 get merged it will (probably) create a merge conflict.

Indeed it does. Could you look at it please, and then we can merge

@pedr
Copy link
Collaborator Author

pedr commented Mar 10, 2023

Indeed it does. Could you look at it please, and then we can merge

Resolved.

@laurent22 laurent22 merged commit d6d4897 into laurent22:release-2.10 Mar 12, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Mar 12, 2023
Repository owner unlocked this conversation Mar 12, 2023
@laurent22
Copy link
Owner

@pedr, could you confirm if something needed to be merged in this PR? Because the createSyntheticClipboardEventWithoutHTML function has been removed from the other PR, so there was actually no changes here

@pedr
Copy link
Collaborator Author

pedr commented Mar 12, 2023

@pedr, could you confirm if something needed to be merged in this PR? Because the createSyntheticClipboardEventWithoutHTML function has been removed from the other PR, so there was actually no changes here

Yes, my mistake. Since we changed from my implementation to the pasteAsText function when calling the onPasteAsText event the bug should be fixed. (pasteAsText already uses clipboard.readText which works in Windows)

@pedr pedr deleted the fix-paste-as-text-windows branch November 10, 2023 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Paste as Text only works with default keybinding on Windows
2 participants