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

Writer: non-breaking spaces cause endless "Revert" state #6285

Open
igregson opened this issue Feb 21, 2024 · 10 comments
Open

Writer: non-breaking spaces cause endless "Revert" state #6285

igregson opened this issue Feb 21, 2024 · 10 comments
Labels
type: bug 🐛 Is a bug; fixes a bug

Comments

@igregson
Copy link

igregson commented Feb 21, 2024

Description

The exact issue I'm seeing is described here (in the issue it was suggested to the OP to create a GH issue but it seems he never did):

https://forum.getkirby.com/t/orange-bar-keeps-re-appearing-on-reverting-changes-in-panel/24271

I experienced the bug in Kirby v3 and am still experiencing it in Kirby v4.

It does not always happen. I have not yet been able to isolate what's needed to recreate it.

It happens often enough to be annoying and potentially quite confusing for users (e.g. resulting in not knowing if their changes were actually reverted).

Expected behavior

When clicking "revert" the "Revert" and "Save" buttons should disappear.

Your setup

Kirby Version
4.1.0

Your system (please complete the following information)

  • Device: Mac Book
  • OS: macOS
  • Browser: Brave
@igregson igregson changed the title "Revert" and "Save" buttons sometimes do not disappear after clicking "Revert" "Revert" and "Save" buttons sometimes still show after clicking "Revert" Feb 21, 2024
@igregson
Copy link
Author

igregson commented Feb 21, 2024

I don't know if it's related, but I've noticed that ?language=null is appended to the key of each localstorage item (which seems to be used for tracking/persisting change state):

CleanShot 2024-02-21 at 17 20 49@2x

UPDATE: This does not seem related (I've just checked on a v3 site which also has this issue and the language query param is not on the key).

@afbora
Copy link
Member

afbora commented Feb 21, 2024

This issue was related to special characters. If you share the content file of the page you have the issue, maybe we can replicate the issue and find a solution.

@afbora afbora added the needs: replication 🔬 Requires a sample to reproduce the issue label Feb 21, 2024
@igregson
Copy link
Author

igregson commented Feb 22, 2024

This issue was related to special characters.

So it's a known issue?

Any further context to point to here (other issues, PRs) (I've looked but haven't been able to find any).

If you share the content file of the page you have the issue, maybe we can replicate the issue and find a solution.

Unfortunately it's within the context of a closed source project.

I'll try to create an isolated test case that reproduces it.


It's worth pointing out the following workaround (that suggests this isn't related to special chars in the content file):

  1. On a page with the issue...
  2. Reload the page (i.e. click the browser's "refresh" button)
  3. Before making any content changes, click the "Revert" button
  4. It works (no issue)

However, if you make changes before clicking the "Revert" button it does not work.

@igregson
Copy link
Author

Alright, read through some of the past context and found the following:

  • I'm only seeing the issue when changes are made inside a text block (i suppose any type of input field that uses prosemirror)
  • The issue seems to be from   getting inserted (from pasted content). It's not visible in the editor. I've noticed these characters so far only between sentences when the author used double spaces between them.
  • The site I'm seeing this on has lots of migrated content from a legacy WP site. There's lots of poorly formatted HTML that includes   in the plain text source.

@igregson
Copy link
Author

igregson commented Feb 22, 2024

Seems the primary PR that previously addressed this (#3758) was mostly around  .

I can reliably reproduce this by pasting HTML that contains   into a writer field instance.

So to reproduce:

  1. Open a page with a writer field input
  2. Paste html into it that contains   (tried to add this here but it seems GitHub's markdown handling strips it)
  3. Save
  4. Reload
  5. Make a change in the field
  6. Click the "Revert" button
  7. Notice that the "Change" and "Save" buttons are still there

Question:

Is there any reason to preserve   instances in pasted text? GitHub seems to be stripping it.

Maybe the simplest "fix" is to replace all   instances with actual spaces on paste. To handle existing content in this state the same "replace" approach could be done on save (as a sanitization step).

@distantnative distantnative added type: bug 🐛 Is a bug; fixes a bug and removed needs: replication 🔬 Requires a sample to reproduce the issue labels Mar 23, 2024
@distantnative
Copy link
Member

I could replicate it as described, @igregson, thanks.

What It seems to be:
After saving and reloading, our backend sends the field value with a normal space or actual non-breaking space (probably the latter, but not sure yet). But ProseMirror will return it after the input as  . So our content store is comparing those strings against each other and determine that they are not equal, though changed. This is why hitting "Revert" is not fixing it as there wills till be the normal (non-breaking) space vs. the encoded &nbsp:.

@distantnative distantnative changed the title "Revert" and "Save" buttons sometimes still show after clicking "Revert" Writer: non-breaking spaces cause endless "Revert" state Mar 23, 2024
@distantnative
Copy link
Member

@bastianallgeier @lukasbestle How can we preserve &nbsp: when the text is processed by Parsley?

@lukasbestle
Copy link
Member

TBH I can't really help here as I wasn't involved in the development of Parsley.

@distantnative
Copy link
Member

@lukasbestle My mistake, I always mix up Parsley and Sane there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug 🐛 Is a bug; fixes a bug
Projects
None yet
Development

No branches or pull requests

4 participants