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

Handle HTML escape on paste and type #2348

Merged
merged 6 commits into from
Sep 3, 2024

Conversation

finkef
Copy link
Collaborator

@finkef finkef commented Sep 2, 2024

Fixes #1617.

This PR escapes HTML on type and on paste with text/plain. Paste with text/html preserves HTML formatting. In favor of supporting OCR functionality, we've kept the previous escape strategy if OCR is detected.

In order to test this functionality properly, puppeteer requires access to the browser clipboard. This unfortunately is only possible with pages served via HTTPS. To account for this, we run a local ssl proxy that proxies 3000 -> 3001 with a self-signed certificate (which is ignored by Browserless).

Copy link
Contributor

@raineorshine raineorshine left a comment

Choose a reason for hiding this comment

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

This is just a visual code review. Will test behavior when I'm at my computer tomorrow. Thanks!

src/e2e/puppeteer/__tests__/escape-html.ts Outdated Show resolved Hide resolved
src/components/Editable.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@raineorshine raineorshine left a comment

Choose a reason for hiding this comment

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

I tested this and it works great for the three cases specified in the issue. Thanks!

The only weirdness I encountered was copying and pasting HTML back into em. I was a little surprised to find that the escaped HTML that was typed got converted into unescaped HTML upon pasting. I was able to use the browser's "Paste and Match Style" command (i.e. paste as plain text) in order to paste the escaped HTML, however in my experience less technical users are often not aware of this special paste command.

To be more precise:

4. Copy and paste HTML from em

Steps to reproduce

  1. Type hello <b>world</b> into em.
  2. Select the text and copy to clipboard.
  3. Delete the thought.
  4. Paste into a new thought.

Current Behavior

The originally escaped HTML is now unescaped. The thought gets rendered as "hello world".

Expected Behavior

Should copying and pasting text from em always result in the same text? Does the typical user know how to use Paste and Match Style? Open to your thoughts.

@finkef
Copy link
Collaborator Author

finkef commented Sep 3, 2024

This was due to an unescape of the HTML when pasting which needed to be removed.

This introduced a regression in an importText test case that imports a special <li> sequence from OSX Notes. However, the test expected a paste as text/plain which should not be unescaped. The test has been removed.
Pasting from OSX Notes as text/html works as expected.

@raineorshine raineorshine merged commit e488361 into cybersemics:main Sep 3, 2024
3 checks passed
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.

Encode HTML on edit
2 participants