-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
[2.0.3] getSemanticHTML
is broken
#4509
Comments
Introduced wtih Commit 07b68c9. The end result of replacing all spaces with An alternative implementation of editor.js converHTML() that preserves whitespace might replace all but one consecutive space character with be: if (blot instanceof TextBlot) {
const escapedText = escapeText(blot.value().slice(index, index + length));
return escapedText.replaceAll(/ +/g, (match) => " ".repeat(match.length - 1) + " ")
} A work around (which replaces the last consecutive quill.getSemanticHTML().replaceAll(/((?: )*) /g, '$1 ') |
I think this is bad. Sometimes I need to write Can we revert to the amazing |
I think @luin can help us here Thanks |
This is quite critical bug to our project. This changes how the values are saved to database. Luckily this didn't hit production before noticing. |
We have the same issue please revert this change. Don't want to save nbps to the database. |
Spent the entire day only to discover this is a known and unaddressed bug. The problem with all spaces becoming |
Could this be a configuration option? So when someone needs it to be "normal" spaces it can? |
commenting to follow, hoping to see an update here. |
IMHO this is critical bug |
Original author: slab#4509 (comment) Refs: slab#4509
Completely destroys displaying edited text for me, since text now fails to wrap. Please revert. |
Yes. Very much so. For example, ngx-quill (https://github.com/KillerCodeMonkey/ngx-quill) also suffers from this. If the editor is used as part of Angular reactive forms, the value of the form control has these added characters after user has finished typing something with whitespaces. |
Any update on this @luin? |
The fact that this hasn't been addressed yet has me concerned that I'm not...using Quill correctly, or something. This respectfully seems like an incredibly critical bug. Is there something I'm missing? |
Yes, it's under the name of |
Not too sure about this. Biggest pro of Quill is that it's free, not that it's feature complete. If I have to pay for it, there's much better payed alternatives, eg. Freola. I'm rather concerned that Slab is abandoning it. There's 400+ open issues, 60 open PRs and not really much to be seen from the maintainers |
Don't be dramatic, just stay on version 2.0.2 for now, until they can fix it. Before version 2, the repository was inactive for over 4 years. It's only been 2-3 months since version 2.0.3 |
Yeah. Not another drama please. |
Jeez this thread is a joke. Just do this in your code: |
Probably you are. Most comments complain that this bug has them save |
Or, @DirkLachowski, since there are so many of us here, our way of using Quill is not wrong either. 🥰 |
Well, "correct" is literally there in @KyeRussell 's question, I just reused that wording when answering it. And I beg to disagree, there's an intended way to use Quill. From the Delta readme:
The real problem is that there are not that much Delta to whatever render tools, so people resort to Quill as a Delta renderer. That works sometimes, but it's not a great idea. |
@islandmyst Your solution works for me. Thank you for posting that! |
If that is the case, then why is
An end user who presented with wrapping lines in an editor intends to have lines wrap. Swapping for If the fix is using the Delta system, then If the fix is to revert 07b68c9 and allow |
@TheSeg Tell that the maintainers, not me. If you think that that function needs a fix, you can open a PR and we'll see if it will be merged. That said, That said: Using the Delta format as a representation for content and changes is just a big read flag showing lack of upfront design. So, there's maybe more errors to come and it's time to move on. I'll do so. |
I opened a pull request with a fix. Hopefully it will get approved. |
Thank you @stashaway and I hope it gets approved as well |
The PR is this: #4587. |
I created an alternative fix (PR #4598) that introduce options to getSemanticHTML, so it can be used as it is (with |
Any updates? |
Yikes.. breaking backwards compatibility in a patch version... Guess the one take-away we got from this is to fix quill to whatever version works and leave it there until we hit critical vulnerabilities... |
QuillJS 2.0.3 changed the way semanticHTML is rendered by replacing all spaces with .. what an absolute joke.. slab/quill#4509
this must be some psyop. trust no patch. |
I think a patch should be based on the suggested fix proposed by @islandmyst (#4509 (comment)) |
Steps for Reproduction
Expected behavior:
The text in the editor is
a text with multiple spaces
.getSemanticHTML
returns<p>a text with multiple spaces</p>
. Which is wrong - the semantics of the text has changed.Actual behavior:
See above.
Platforms:
Any.
Version:
2.0.3
The text was updated successfully, but these errors were encountered: