Repository navigation
Stop getSemanticHTML from turning every space into - #4827
Closed
afonsojanu wants to merge 1 commit into
Closed
afonsojanu wants to merge 1 commit into
afonsojanu wants to merge 1 commit into
Conversation
convertHTML replaced every single space in a text blot with , which changes the actual meaning of the text: "a text with multiple spaces" came back with a non-breaking space between every word instead of the plain spaces that were actually typed. A run of two or more consecutive spaces genuinely needs at least one non-breaking space, since HTML would otherwise collapse the whole run down to a single visible space when rendered. A single space between two words doesn't have that problem and should stay as a plain space. Fixes #4509.
Author
|
Closing this one too, same reason: starting fresh and clearing out everything I had open. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4509.
convertHTML() in editor.ts replaces every space in a text blot with
, which is whygetSemanticHTML()comes back with a non-breaking space between every single word instead of the plain spaces that were actually typed. That changes the meaning of the text, which is exactly what the issue is complaining about.The reason the nbsp substitution exists at all is that HTML collapses a run of consecutive whitespace down to one visible character, so if you have two or three spaces in a row you do need at least one nbsp to keep them all visible. A single space between two words has never had that problem, so there's no reason to touch it.
Changed the replace to only kick in on runs of two or more consecutive spaces, keeping one of them as a plain space and turning the rest into nbsp. A lone space is left untouched.
I couldn't get the project's own browser-based test suite (Playwright/Chromium via vitest) to actually launch in the environment I was working in - Chromium installs fine but the browser process gets killed immediately on launch, unrelated to anything in this change. I checked the transform logic directly with a small standalone script instead and it does what's expected:
I also added two cases to the existing getSemanticHTML() describe block in quill.spec.ts covering the single-space and multi-space scenarios, written in the same style as the tests already there, but wasn't able to confirm they pass against the real suite for the reason above. Happy to double check that part if someone can run it against the actual browser suite.