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

Encode HTML on edit #1617

Closed
3 tasks
raineorshine opened this issue Jul 7, 2022 · 0 comments · Fixed by #2348
Closed
3 tasks

Encode HTML on edit #1617

raineorshine opened this issue Jul 7, 2022 · 0 comments · Fixed by #2348
Assignees
Labels
bug Something isn't working

Comments

@raineorshine
Copy link
Contributor

raineorshine commented Jul 7, 2022

Currently, basic HTML formatting tags (b, i, strong, em) are allowed to be typed into a thought and rendered as HTML. All other tags are stripped out. In practice, this has not been very useful, and has gotten in the way of typing literal representations of HTML.

Instead, encode the HTML as character entities so it is rendered literally.

Handling of HTML in pasted content depends on the content-type:

  • When the pasted content-type is text/html, we should render the formatting tags as HTML as done currently.
  • When the pasted content content-type is text/plain but contains HTML (i.e. htmlText is empty and isHTML is true), we should escape the HTML and render character entities.

Three cases:

  • Type HTML: Escape HTML and render literal character entities (changed behavior)
  • Paste text/plain with HTML: Escape HTML and render literal character entities (changed behavior)
  • Paste text/html: Render formatting tags (current behavior)

It is very likely that fixing one could break another if you are not careful. Therefore, it is recommended that you test all three cases after fixing one to avoid a regression. We currently do not have testing logic to mock pasting text with different content-types. You could try to mock e.clipboardData.getData, or it is acceptable to just do manual testing.

1. Type HTML

Steps to Reproduce

  1. Create a new thought
  2. Type hello <b>world</b>
  3. Hit escape or move the cursor

Current Behavior

The HTML is NOT escaped. The thought gets rendered as "hello world".

Expected Behavior

Typed angled brackets should be escaped, i.e. converted to character entities.

The thought's value should be hello &lt;b&gt;world&lt;/b&gt;

Which should get rendered literally as hello <b>world</b>

2. Paste text/plain

Steps to reproduce

  1. Type hello <b>world</b> in your text editor.
  2. Copy hello <b>world</b> from your text editor. This will ensure text/html is not added to the clipboard.
  3. Paste into a new thought

Current Behavior

The HTML is NOT escaped. The thought gets rendered as "hello world".

Expected Behavior

Same as typing text/plain.

The thought's value should be hello &lt;b&gt;world&lt;/b&gt;

Which should get rendered literally as hello <b>world</b>

3. Paste text/html

Steps to Reproduce

Copy and paste this text into a new thought: hello world
(Copying the rendered text from the browser will copy the HTML into the text/html part of the clipboard.)

Current Behavior

The HTML is not escaped. The thought gets rendered as "hello world".

Expected Behavior

The current behavior should not change.

Testing

Add tests for each case.

I'm not sure what testing level to go for here: importText reducer or RTL tests, or a combination. Prefer reducer tests when possible.

@raineorshine raineorshine added the bug Something isn't working label Jul 7, 2022
@raineorshine raineorshine added this to the 🧠 Fluid Sensemaking I milestone Jul 7, 2022
@raineorshine raineorshine changed the title Escape HTML on edit Encode on edit Jul 7, 2022
@raineorshine raineorshine changed the title Encode on edit Encode HTML on edit Jul 7, 2022
@ankitkarna99 ankitkarna99 self-assigned this Dec 30, 2022
@cybersemics cybersemics deleted a comment from ankitkarna99 Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants