Can't delete embed blots on android

[EthanRutherford]

Steps for Reproduction

1. Visit https://codepen.io/EthanRutherford/pen/LQqbRE on android
2. focus the editor
3. hit delete

Expected behavior:
the embedded blot is deleted

Actual behavior:
the editor is defocused, and the blot is not deleted

Platforms:
android O, chrome

Version:
1.3.5

[charrondev]

I did a little bit of testing myself and can confirm this on a few different platforms as well.

I was able to reproduce on Android versions 6->8.1 using Chrome 64 and the google keyboard. I was unable to reproduce on Samsung devices however (Chrome 64 and android 7.1, although these devices were not using the Google keyboard).

I also able to reproduce this issue on the same devices on the quill homepage https://quilljs.com. The same behaviour occurs with the formula embed in the main example.

[EthanRutherford]

I can also reproduce with the formula embed on the homepage

[ivanalejandro0]

I've stumbled upon this problem as well.
It seems that the Gboard keyboard behaves differently than others.
To workaround this problem I redefined the update method on my custom blot that extends Embed to handle the childList case where a node is removed.

You can see a working example here: https://codepen.io/ivanalejandro0/pen/GdOBjQ

Here's a copy/paste of the code that does the trick:

const Base = Quill.import('blots/embed');
class MentionBlot extends Base {
  // ... more code here ...
  /**
   * Redefine the `update` method to handle the `childList` case.
   * This is necessary to correctly handle "backspace" on Android using Gboard.
   * It behaves differently than other cases and we need to handle the node
   * removal instead of the `characterData`.
   */
  update(mutations, context) {
    // `childList` mutations are not handled on Quill
    // see `update` implementation on:
    // https://github.com/quilljs/quill/blob/master/blots/embed.js

    mutations.forEach(mutation => {
      if (mutation.type != 'childList') return;
      if (mutation.removedNodes.length == 0) return;

      setTimeout(() => this._remove(), 0);
    });

    const unhandledMutations = mutations.filter(m => m.type != 'childList')
    super.update(unhandledMutations, context);
  }
  
  _remove() {
    // NOTE: call this function as:
    // setTimeout(() => this._remove(), 0);
    // otherwise you'll get the error: "The given range isn't in document."
    const cursorPosition = quill.getSelection().index - 1;

    // see `remove` implementation on:
    // https://github.com/quilljs/parchment/blob/master/src/blot/abstract/shadow.ts
    this.remove();
    
    // schedule cursor positioning after quill is done with whatever has scheduled
    setTimeout(() => quill.setSelection(cursorPosition, Quill.sources.API), 0);
  }
  // ... more code here ...
}

I've tried this with latest quill (1.3.6 as today), on Android 7.0, Chrome 66.0.3359.158. I've tested it with both Gboard and Samsung keyboards.

I think that this fix can be ported to the update method here https://github.com/quilljs/quill/blob/master/blots/embed.js#L67 but I didn't have time to go there.
Hopefully my workaround and example can give you some insight of the problem and how to workaround/fix it.

[lixiaoyan]

@ivanalejandro0
I have run into a problem with the code above, and after some digging, I found a way to resolve it.

      if (mutation.type === 'childList' && mutation.removedNodes.length === 1) {
        if (
          mutation.removedNodes[0] === this.leftGuard ||
          mutation.removedNodes[0] === this.rightGuard
        ) {

If you modified the dom inside embeds (specifically, removeChild), the embed may be removed unexpectedly. We should check whether the dom removed is one of the guard nodes first.

[sgtpepper43]

I'm running into this problem, and while the above code allows me to delete the embed, it also dismisses the keyboard.

[mahdiG]

I used @ivanalejandro0 's solution and it worked. I had to blur and focus the element to reopen the keyboard.

[laukaichung]

I have to add a zero-width no break space character (\uFEFF) next to my non-contenteditable embed block (not 'blots/embed' but blots/block/embed) along with @ivanalejandro0 solution in order to get it work on Android devices.

Custom Blot:

import React from "react"
import { render } from "react-dom";

export class TextBlot extends EmbedPatch {
  static blotName = "textBlot";
  static tagName = "div";
  static create(values) {
    const domNode = super.create(values);
    render(
      <React.Fragment>
        <div contenteditable={false}>
            {values.text}
        <div>
        {"\uFEFF"}
      </React.Fragment>,
      domNode
    );
    domNode.dataset = values;
    return domNode;
  }

}

import { Quill } from "react-quill";
const Embed = Quill.import("blots/block/embed");

export default class EmbedPatch extends Embed {
   // ivanalejandro0's methods 
}

The extended EmbedPatch class is basically the solution by @ivanalejandro0 (#1985 (comment)).

[Alejopdp]

Hi @laukaichung , do you have a full code example? I'm trying to fix this on my React-Quill functional component but i'm not getting it to work. I'm configuring a modules object inside my component to pass it to the real Quill js editor component. Using a blot class kinda crashes with my implementation.

If you could pass me a full example I'd really appreciate it.

Thanks!

[jullicer]

I've stumbled upon this problem as well.
It seems that the Gboard keyboard behaves differently than others.
To workaround this problem I redefined the update method on my custom blot that extends Embed to handle the childList case where a node is removed.

You can see a working example here: https://codepen.io/ivanalejandro0/pen/GdOBjQ

Here's a copy/paste of the code that does the trick:

const Base = Quill.import('blots/embed');
class MentionBlot extends Base {
  // ... more code here ...
  /**
   * Redefine the `update` method to handle the `childList` case.
   * This is necessary to correctly handle "backspace" on Android using Gboard.
   * It behaves differently than other cases and we need to handle the node
   * removal instead of the `characterData`.
   */
  update(mutations, context) {
    // `childList` mutations are not handled on Quill
    // see `update` implementation on:
    // https://github.com/quilljs/quill/blob/master/blots/embed.js

    mutations.forEach(mutation => {
      if (mutation.type != 'childList') return;
      if (mutation.removedNodes.length == 0) return;

      setTimeout(() => this._remove(), 0);
    });

    const unhandledMutations = mutations.filter(m => m.type != 'childList')
    super.update(unhandledMutations, context);
  }
  
  _remove() {
    // NOTE: call this function as:
    // setTimeout(() => this._remove(), 0);
    // otherwise you'll get the error: "The given range isn't in document."
    const cursorPosition = quill.getSelection().index - 1;

    // see `remove` implementation on:
    // https://github.com/quilljs/parchment/blob/master/src/blot/abstract/shadow.ts
    this.remove();
    
    // schedule cursor positioning after quill is done with whatever has scheduled
    setTimeout(() => quill.setSelection(cursorPosition, Quill.sources.API), 0);
  }
  // ... more code here ...
}

I've tried this with latest quill (1.3.6 as today), on Android 7.0, Chrome 66.0.3359.158. I've tested it with both Gboard and Samsung keyboards.

I think that this fix can be ported to the update method here https://github.com/quilljs/quill/blob/master/blots/embed.js#L67 but I didn't have time to go there.
Hopefully my workaround and example can give you some insight of the problem and how to workaround/fix it.

I tried putting your code either on quill/blots/embed or quill-mention/blots/mention but error stating quill is undefined.

[jullicer]

i updated the blot file and added update method

if user is removing the guard i reset the blot to text so the mention plugin will trigger again.

[oller]

i updated the blot file and added update method

if user is removing the guard i reset the blot to text so the mention plugin will trigger again.

    update(mutations, context) {
        mutations.forEach((mutation) => {
            if (
                mutation.type === "childList" &&
                (Array.from(mutation.removedNodes).includes(this.leftGuard) ||
                    Array.from(mutation.removedNodes).includes(this.rightGuard))
            ) {
                let tag;
                if (mutation.previousSibling) {
                    tag = mutation.previousSibling.innerText;
                } else if (mutation.nextSibling) {
                    tag = mutation.nextSibling.innerText;
                }
                if (tag) {
                    super.replaceWith("text", tag);
                }
            }
        });

        super.update(mutations, context);
    }

Thanks for the solution @jullicer, just typing it up, might save someone else the trouble!

[xuergo]

i updated the blot file and added update method

if user is removing the guard i reset the blot to text so the mention plugin will trigger again.

Hello brother, where should I put it?

[wpj-wpj-wpj]

I've oppened PR #4476

the reason of this issue

the cursor focus between the contentNode of the embed and the rightGuard of the embed which is a zero-width space

in this case, the native selection range is like

{
 start:{
   node: rightGuard,
   offset: 0,
 }
}

until now, everything is fine

but when quill normalize this range, it goes the wrong way, because offset is 0.

the final range we get is the position before our embed

so when you backspace

you can't delete the embed

you just delete the previous sibling