using beforeinput instead of DOMSubtreeModified to make Gmail's composer faster
Categories
(Web Compatibility :: Desktop, defect)
Tracking
(firefox111 affected)
Tracking | Status | |
---|---|---|
firefox111 | --- | affected |
People
(Reporter: masayuki, Assigned: twisniewski)
References
(Blocks 1 open bug)
Details
(Keywords: webcompat:site-wait)
Attachments
(1 file)
(deleted),
image/png
|
Details |
- Open Gmail.
- Click "Compose" to open email composer.
- Right click on the email body editor and choose "Inspect Element".
- 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.
Updated•4 years ago
|
Reporter | ||
Comment 2•3 years ago
|
||
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.
Comment 3•3 years ago
|
||
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 afunction
_.rs
istrue
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 theHTMLDocument
athttps://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()))
};
Reporter | ||
Comment 4•3 years ago
|
||
Thank you, Karl-san, that sounds like that:
beforecut
can be replaced withbeforeinput
event whoseinputType
isdeleteByCut
beforepaste
can be replaced withbeforeinput
event whoseinputType
isinsertFromPaste
orinsertFromPasteAsQuotation
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...
Reporter | ||
Comment 5•3 years ago
|
||
Ah, not one line patch, they also need to touch the event listener (this.xG
).
Comment 6•3 years ago
|
||
(just for information) for firebugIgnore
context.
https://github.com/google/closure-library/blob/c7ea3f4a00d81a3215a76f9085472e7b36166019/closure/goog/editor/field.js#L1538-L1554
Comment 7•3 years ago
|
||
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.
Reporter | ||
Comment 9•2 years ago
|
||
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.
Updated•2 years ago
|
Comment 10•2 years ago
|
||
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
Reporter | ||
Comment 11•1 years ago
|
||
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.
Updated•1 years ago
|
Comment 12•1 year ago
|
||
This should now be fixed. See this comment for more details: https://github.com/mozilla/standards-positions/issues/807#issuecomment-1702977884
Reporter | ||
Comment 13•1 year ago
|
||
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?
Comment 14•1 year ago
|
||
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?
Description
•