quill icon indicating copy to clipboard operation
quill copied to clipboard

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

Open chearon opened this issue 7 years ago • 14 comments

QUICK FIX: https://github.com/quilljs/quill/issues/2096#issuecomment-399576957

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.

chearon avatar Apr 30 '18 21:04 chearon

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

Ethan-G avatar Jun 08 '18 11:06 Ethan-G

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 avatar Jun 21 '18 22:06 foleyatwork

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.

foleyatwork avatar Jun 22 '18 17:06 foleyatwork

Brilliant!! Awesome work @foleyatwork!

chearon avatar Jun 22 '18 17:06 chearon

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

chearon avatar Jun 22 '18 17:06 chearon

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.

foleyatwork avatar Jun 22 '18 17:06 foleyatwork

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);

chearon avatar Jun 22 '18 20:06 chearon

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.

Ethan-G avatar Jul 13 '18 10:07 Ethan-G

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.

chearon avatar Jul 13 '18 13:07 chearon

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.

GJMAC avatar Sep 04 '19 18:09 GJMAC

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);

kamelito78 avatar Nov 19 '20 09:11 kamelito78

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.

KaisoBits avatar Dec 22 '20 20:12 KaisoBits

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?

jlining avatar Apr 06 '21 17:04 jlining

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.

hendrikschneider avatar Jun 11 '22 06:06 hendrikschneider

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

IGMontero avatar Feb 28 '23 13:02 IGMontero

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

Subtletree avatar Feb 28 '23 22:02 Subtletree

👋 Sorry for the late update. We are working on the next release. This is fixed in https://github.com/quilljs/quill/pull/3807. Feel free to let me know if there is anything missing.

luin avatar Jun 23 '23 23:06 luin

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

venugmp avatar Jun 28 '23 13:06 venugmp

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

luin avatar Jul 02 '23 06:07 luin

@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?

wzoubir avatar Aug 16 '23 09:08 wzoubir

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

chearon avatar Aug 16 '23 15:08 chearon