Open Bug 1671072 Opened 4 years ago Updated 1 year ago

using beforeinput instead of DOMSubtreeModified to make Gmail's composer faster

Categories

(Web Compatibility :: Desktop, defect)

x86_64
Windows 10
defect

Tracking

(firefox111 affected)

ASSIGNED
Tracking Status
firefox111 --- affected

People

(Reporter: masayuki, Assigned: twisniewski)

References

(Blocks 1 open bug)

Details

(Keywords: webcompat:site-wait)

Attachments

(1 file)

  1. Open Gmail.
  2. Click "Compose" to open email composer.
  3. Right click on the email body editor and choose "Inspect Element".
  4. Show list of DOM event listeners of the element.

There is DOMSubtreeModified event listener. It is a legacy event of DOM Mutation Events, and once this listener is added, the document works slower with every DOM changes (both modifying the editor with your operation and by JS).

It seems that this is not used when I open it on Chrome. Now, we support beforeinput event on Nightly builds, and it can be checked whether beforeinput event is supported or not with

typeof window.InputEvent.prototype.getTargetRanges === "function"

So, I'd like Google to use beforeinput (if possible) instead of DOMSubtreeModified on Firefox.

Contacted Google through the mailing list.

Summary: Gmail's composer listens to `DOMSubtreeModified` event on Firefox Nightly, but not on Chrome → using beforeinput instead of DOMSubtreeModified to make Gmail's composer faster

Karl-san, do you have replies which explains that it't required for beforeinput or for another incompatible things? If the lattrer, I'll put it into my queue.

Flags: needinfo?(kdubost)

On October 2020, the answer was:

Thanks, I've filed a bug internally and I'll try to find someone who can take a look at this.

I can try to send an email again to remind them and ask them why they have to use DOMSubtreeModified

I checked the source code. And there is only instance of it in the full code.
Trying to understand why they call DOMSubtreeModified.

    _.RX.prototype.nCa = function () {
      if (_.uLc || !this.Mn()) this.Xt.listen(this.Ea(), 'DOMSubtreeModified', this.Cb);
       else {
        var c = this.Fl.Mb();
        this.Xt.listen(c, _.WOh, this.Cb, !0);
        this.Xt.listen(c, _.hga, (0, _.wr) (this.Ac, this, this.Cb), !0)
      }
    };

Ah this is triggered once we pressed on the compose button (pen icon).

called by ZBi.wb.nCa.call(this);
which was called by DY.wb.nCa.call(this);
which was called by

  _.rs
    ? this.nCa()
    : (this.addListener(["beforecut", "beforepaste", _.ji, _.nua], this.xG),
      this.addListener([_.zta, _.rl], _.VKa(this.Gv)),
      this.addListener(_.ji, this.Tb));

in

_.RX.prototype.Dxa = function () {
  _.Sq && _.Akc(this.Fl.getWindow());
  _.zM != this.id && this.execCommand(_.MCa);
  _.Sq
    ? (this.addListener(_.Ii, this.SPb), this.addListener(_.Ji, this.PPb))
    : this.addListener(_.Ii, this.RPb);
  this.addListener(_.Fg, this.Fua, _.rs);
  _.rs
    ? this.nCa()
    : (this.addListener(["beforecut", "beforepaste", _.ji, _.nua], this.xG),
      this.addListener([_.zta, _.rl], _.VKa(this.Gv)),
      this.addListener(_.ji, this.Tb));
  this.addListener(_.ss ? _.nua : "dragdrop", this.Tb);
  this.addListener(_.Uj, this.Jc);
  this.addListener(_.Vj, this.Pc);
  this.addListener(_.Wj, this.Qc);
  this.QBa = new _.tB(this.B6c, 250, this);
  this.kb(this.QBa);
  this.$b && this.addListener(_.nh, VOh);
  this.addListener(_.Bk, this.Sjc);
  this.Zub
    ? (this.Xt.listen(this.Fl.Mb(), _.Jk, this.Vxb),
      this.addListener(_.gi, this.Rjc))
    : this.addListener(_.Jk, this.Vxb);
  this.Ea();
  this.LZa();
  this.dQ();
  this.dispatchEvent(_.lk);
  for (var c in this.Aa) this.Aa[c].enable(this);
};

in this.addListener(_.Fg, this.Fua, _.rs);

  • _.Fg is "blur"
  • this.Fua is a function
  • _.rs is true

if (_.uLc || !this.Mn()) this.Xt.listen(this.Ea(), 'DOMSubtreeModified', this.Cb);

_.uLc is false but !this.Mn() is true

what is this.Mn() always returns false.

The second part of the statement which is not adding DOMSubtreeModified

    : (this.addListener(["beforecut", "beforepaste", _.ji, _.nua], this.xG),
      this.addListener([_.zta, _.rl], _.VKa(this.Gv)),
      this.addListener(_.ji, this.Tb));

let's see what is that

    : (this.addListener(["beforecut", "beforepaste", "drop", "dragend"], this.xG),
      this.addListener(["cut", "paste"], _.VKa(this.Gv)),
      this.addListener("drop", this.Tb));

Ah these are still supported in Chrome.
https://bugs.chromium.org/p/chromium/issues/detail?id=690135

It was removed 14 years ago in Gecko cf Bug 418457

And the other part for the if on DOMSubtreeModified what would happen if the else road was taken instead.

      if (_.uLc || !this.Mn()) this.Xt.listen(this.Ea(), 'DOMSubtreeModified', this.Cb);
       else {
        var c = this.Fl.Mb();
        this.Xt.listen(c, _.WOh, this.Cb, !0);
        this.Xt.listen(c, _.hga, (0, _.wr) (this.Ac, this, this.Cb), !0)
      }
  • c is the HTMLDocument at https://mail.google.com/mail/u/0/#drafts?compose=new
        this.Xt.listen(c, [ "DOMNodeInserted", "DOMNodeRemoved", "DOMNodeRemovedFromDocument", "DOMNodeInsertedIntoDocument", "DOMCharacterDataModified" ], this.Cb, !0);
        this.Xt.listen(c, "DOMAttrModified", (0, _.wr) (this.Ac, this, this.Cb), !0)

this.Cb huh. firebugIgnore?

    _.RX.prototype.Cb = function (c) {
      this.aE(_.dh) || (c = c.Kf ? c.Kf() : c, c.target.firebugIgnore || (this.Ya = this.Ka = !0, this.wa.start()))
    };
Flags: needinfo?(kdubost)

Thank you, Karl-san, that sounds like that:

  • beforecut can be replaced with beforeinput event whose inputType is deleteByCut
  • beforepaste can be replaced with beforeinput event whose inputType is insertFromPaste or insertFromPasteAsQuotation

So, _.rs can be true for them if InputEvent.getTargetRanges is not undefined (onbeforeinput does not work on Blink 😢). And the other events are supported on Gecko too. So, it could be fixed by one line patch...

Ah, not one line patch, they also need to touch the event listener (this.xG).

onbeforeinput in chrome they seem they were waiting for the spec to be modified. It has been done in https://github.com/whatwg/html/pull/6743 but not yet fully merged. I pinged the person on the PR to check the comment from domenic.

Then they would still need to implement it.

Masayuki is this still reproducible for you?

Flags: needinfo?(masayuki)

Yeah, I still hit a breakpoint of DOMSubtreeModified event listener in the debugger. Just listening to a DOM mutation event causes making the web app slower.

Flags: needinfo?(masayuki)
Assignee: nobody → twisniewski
Status: NEW → ASSIGNED
Attached image DOMSubtreeModified Nightly.png (deleted) —

The issue is still reproducible on the latest Nightly.

Tested on:
Operating system: Windows 10
Browser/Version: Firefox Nightly 111.0a1 (2023-01-25) / Chrome 109.0.5414.75

I realize that when I type something with IME in Google Docs, I see the following warning a lot.

[Child 26032, Main Thread] WARNING: '!compositionStartPoint.IsSet()', file M:/src/editor/libeditor/HTMLEditSubActionHandler.cpp:1217

This means that Google Docs overrides text node update for composing text with a DOM mutation event listener. Therefore, Google Docs uses it too.

This should now be fixed. See this comment for more details: https://github.com/mozilla/standards-positions/issues/807#issuecomment-1702977884

I still see breaking at DOMSubtreeModified event listener at this code when I enable event listener break points of the DevTools. (I just typed a character in the composer.)

      Meb = function () {
        var a = Oeb,
        b = function (c) {
          return a.call(b.src, b.listener, c)
        };
        return b
      };

Is this caused by the rollout timing or unexpected case for you?

Flags: needinfo?(masonfreed)

My guess is that this is just rollout timing. The change just landed last week, so it'll take some time to go live. (I don't know how long, sorry!) But I'd say if you still notice this in ~2 more weeks, let me know?

Flags: needinfo?(masonfreed)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: