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

Spell check deletes words if there's color formatting on the line #2096

Closed
chearon opened this issue Apr 30, 2018 · 21 comments
Closed

Spell check deletes words if there's color formatting on the line #2096

chearon opened this issue Apr 30, 2018 · 21 comments

Comments

@chearon
Copy link

chearon commented Apr 30, 2018

QUICK FIX: #2096 (comment)

Steps for Reproduction

  1. Visit quilljs.com and go to the third example where you can set a font color
  2. Create a colored sentence with a typo, like "I love makig bugs"
  3. Right click "makig" to correct the spelling

Expected behavior:
The spelling should be corrected just like regular, non-formatted text

Actual behavior:
Characters are deleted. Sometimes it's just the word, sometimes it's everything before it

Platforms:
macOS High Sierra. Chrome and Safari (notably, not Firefox)

Version:
1.3.6

Related:

The issue doesn't happen when you use classes instead of inline styles, which makes me think it's some kind of issue with normalized, rgb() values and quill uses hex. I tried forcing the Delta to use rgb() but it didn't help.

@Ethan-G
Copy link

Ethan-G commented Jun 8, 2018

I've just encountered this as well. It seems to happen when there's any styling, not just colour.

@foleyatwork
Copy link

Just here to say I'm experiencing this too. Gonna see if I can find a fix but this is a pretty troublesome bug.

@foleyatwork
Copy link

foleyatwork commented Jun 22, 2018

Hey, guys... I found a solution. So here's the breakdown of what's happening:

Quill's color format puts out code like this:

<span style="color: rgba(0, 0, 0, 1);">Mispeled word</span>

The browser's spell-check functionality changes that to:

<font style="color: rgba(0, 0, 0, 1);">Mispelled word</font>

Quill doesn't seem to recognize that font tag as a valid color blot so it deletes it (although once could argue that's still a bug in Quill, deleting content should never happen unless done explicitly IMO).

But the fix is quite easy. I just created an additional blot that recognizes both the font tag and the color attribute, this way when Quill comes across it it knows what to do. I haven't had time to put together a demo but here's my code.

[edit]: Use this instead.

Then just import that into your file and use Quill.register. It doesn't even have to be the primary color formatter, it just has to be present.

@chearon
Copy link
Author

chearon commented Jun 22, 2018

Brilliant!! Awesome work @foleyatwork!

@chearon
Copy link
Author

chearon commented Jun 22, 2018

It needs a little more work though:

  • Probably attrName should be set to color so the quill document is standard ({color: "#aaa"})
  • The model gets correctly updated with the color but (in Safari) it still uses the text before the substitution
  • I'm getting some console errors followed by strange behavior where Quill seems to be going out of sync with the contentEditable
  • create never seems to be called

@foleyatwork
Copy link

foleyatwork commented Jun 22, 2018

Good feedback, this could definitely use some improvement, I'm no expert in creating these blots yet. I'll work on refining this. If you've got any code improvements to share, please do.

@chearon
Copy link
Author

chearon commented Jun 22, 2018

This seems to do everything correctly, I'll update it with any fixes:

const Inline = Quill.import('blots/inline');

class CustomColor extends Inline {
  constructor(domNode, value) {
    super(domNode, value);

    // Map <font> properties
    domNode.style.color = domNode.color;

    const span = this.replaceWith(new Inline(Inline.create()));

    span.children.forEach(child => {
      if (child.attributes) child.attributes.copy(span);
      if (child.unwrap) child.unwrap();
    });

    this.remove();

    return span;
  }
}

CustomColor.blotName = "customColor";
CustomColor.tagName = "FONT";

Quill.register(CustomColor, true);

@Ethan-G
Copy link

Ethan-G commented Jul 13, 2018

Nice one!

Is there a reason you didn't do the workaround of using classes instead of inline styles? I'm looking into it as it makes sanitising XSS easier anyway.

@chearon
Copy link
Author

chearon commented Jul 13, 2018

Mostly because we support a continuous range of colors in our app, so I didn't want to generate 256^3 classes. It should totally work if you only allow a set of colors though.

@GJMAC
Copy link

GJMAC commented Sep 4, 2019

This works well, but unfortunately we have block formatting at the P-level that seem to get lost as soon as the code is hit. The block level is being set with the following:

 const qlFontSize = new Parchment.Attributor.Style('ppsize', 'font-size', { scope: Parchment.Scope.BLOCK });
    Quill.register(qlFontSize, true);

which will result in the markup to result in this ( so the font size is set block related in the paragraph)

<div class="ql-editor" data-gramm="false" contenteditable="false" data-placeholder="Double click to type here">
  <p style="font-family: PPSans; font-size: 11pt;">
     <span style="color: rgb(0, 0, 0);">123 Address</span>
  </p>
  <p style="font-family: PPSans; font-size: 10pt;">
     Paragraph Content with <span style="color: rgb(255, 0, 0);">tyypo</span>.
  </p>
</div>

As soon as the typo is corrected, the text shows fine yet it clears the style from the paragraph. This results in:

<div class="ql-editor" data-gramm="false" contenteditable="false" data-placeholder="Double click to type here">
  <p style="font-family: PPSans; font-size: 11pt;">
     <span style="color: rgb(0, 0, 0);">123 Address</span>
  </p>
  <p style>
     Paragraph Content with <span style="color: rgb(255, 0, 0);">typo</span>.
  </p>
</div>

Any help to avoid losing the block-level formatting is appreciated.

@kamelito78
Copy link

Here another version of @chearon that keeps font-family :

const Inline = Quill.import('blots/inline');

class CustomAttributes extends Inline {
  constructor(domNode, value) {
    super(domNode, value);

    const span = this.replaceWith(new Inline(Inline.create()));

    span.children.forEach(child => {
      if (child.attributes) child.attributes.copy(span);
      if (child.unwrap) child.unwrap();
    });

   // here we apply every attribute from <font> tag to span as a style
    Object.keys(domNode.attributes).forEach(function (key) {
      
        if (domNode.attributes[key].name != "style")
        {
          var value = domNode.attributes[key].value;
          var name = domNode.attributes[key].name;
          if (name == "face")
            name = "font-family";
          span.format(name, value);
        }
  });

    this.remove();

    return span;
  }
}

CustomAttributes.blotName = "customAttributes";
CustomAttributes.tagName = "FONT";

Quill.register(CustomAttributes, true);

@KaisoBits
Copy link

It's still a problem. I think it's worth fixing instead of relying on programmers to create workarounds. Spell correcting is a pretty basic and commonly used feature.

@jlining
Copy link

jlining commented Apr 6, 2021

Hello, I am here as this issue actually is impacting a client who is using custom rich text fields in a Salesforce org. In Salesforce Lightning Experience, they use the open-source Quill Library for the rich text editors in custom fields. The issue occurs if they change the font to anything other than the default font.

There's currently a known issue that is related to this in Salesforce, however, this Known issue is considered as No Fix as it was found that the root cause is an issue with Quill code and not a Salesforce issue.

https://trailblazer.salesforce.com/issues_view?id=a1p3A000001YmYBQA0

I see that this issue has been around since at least 2018. Is there any idea as to what this would be resolved?

@hendrikschneider
Copy link

I want to add to @LeeMustache comment that safari on OSX also uses some autocomplete functionality, which will have the same effect. Basicly, what happen is that the text disappears while the user is till typing and they don't even know what is happening. I've used these workarounds, which seem to be working but it is necesarry that autocomplete/autocorrect will be handled correctly.

@IGMontero
Copy link

What's the state of things with this?
Can't believe it's been 5 years and hasn't been fixed..

@Subtletree
Copy link

What's the state of things with this? Can't believe it's been 5 years and hasn't been fixed..

quill is pretty much dead

@luin
Copy link
Member

luin commented Jun 23, 2023

👋 Sorry for the late update. We are working on the next release. This is fixed in #3807. Feel free to let me know if there is anything missing.

@luin luin closed this as completed Jun 23, 2023
@venugmp
Copy link

venugmp commented Jun 28, 2023

@luin when will be the next release date, that this bug fix is going live. please let us know.

@luin
Copy link
Member

luin commented Jul 2, 2023

@venugmp I don't have a timeline yet but we are working actively on the next release.

@wzoubir
Copy link

wzoubir commented Aug 16, 2023

@chearon @kamelito78 thank you for the fix. However, I'm using Angular, and I placed the code you provided in node_modules/quill/dist/quill.js. Now, I need to deploy the application. Should I put it in another location or just commit node_modules? What are your thoughts?

@chearon
Copy link
Author

chearon commented Aug 16, 2023

You can put that anywhere, e.g. at the top of a module that uses Quill.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests