1288640 - Stop dispatching "text" event in web contents

Status: RESOLVED FIXED

(Core :: DOM: Events, defect, P3)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Using "text" events in web contents does not make sense because it's a non-standard event and doesn't provide enough information.

So, I think that we should stop dispatching it in web contents.

However, mOnlySystemGroupDispatchInContent is slow. So, I think that we should add new flag mOnlySystemGroupDispatch which skips to dispatch event in default event group on any nodes.

Any ideas?

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #0)

> However, mOnlySystemGroupDispatchInContent is slow. 
It is? Why? 
(I'm not aware it being slow)


I think we need to add some telemetry whether pages are using the event. Right now we know nothing about its usage.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Olli Pettay [:smaug] from comment #1)
> (In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #0)
> 
> > However, mOnlySystemGroupDispatchInContent is slow. 
> It is? Why? 
> (I'm not aware it being slow)

See here:
https://dxr.mozilla.org/mozilla-central/rev/4c05938a64a7fde3ac2d7f4493aee1c5f2ad8a0a/dom/events/EventDispatcher.cpp#260,262

While EventDispatcher is dispatching an event in the default group, it needs to check if the node is in chrome document.

Therefore, you requested me to add this comment ;-)
https://dxr.mozilla.org/mozilla-central/rev/4c05938a64a7fde3ac2d7f4493aee1c5f2ad8a0a/widget/BasicEvents.h#113-115

> I think we need to add some telemetry whether pages are using the event.
> Right now we know nothing about its usage.

If it's possible, sounds reasonable, but how? I'm not familiar with telemetry. Do you know some documents about that?

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

bug 1506434 added the telemetry.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Although the percentage of using web apps is not 0, but about only 0.0003%. So, let's disable it on all channels by default and then, if Beta users increase the percentage drastically, we should revert the change.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=df88be70af5cc6effba3e906a6860900f9f53f6b

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ea30f96e05457db22c0b22164427d44d55ba4420

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=99727b772e02b904f51c5a710f169157802e62bb

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a1a47a8af1fbddbb96b2a7ce4156e2531095e163

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1288640 - Make TextComposition not dispatch eCompositionChange events (DOM "text" event) in the default group of web content

The usage of our specific "text" event is enough low (0.0003%).  So, let's
stop dispatching the event in the default group of web content.  Once we
release this new behavior, we can get rid of dispatching the event even in
chrome.  Then, we can optimize the event order for new specs.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8be20508c9d5
Make TextComposition not dispatch eCompositionChange events (DOM "text" event) in the default group of web content r=smaug

Comment 11

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/8be20508c9d5

Comment 12

[Kohei Yoshino [:kohei]]

Posted site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2018/non-standard-text-event-will-no-longer-be-fired-during-ime-composition/

Comment 13

[Daniel Holbert [:dholbert]]

I pushed a followup to fix a minor typo in a code-comment from this bug ("Whehter"):
https://hg.mozilla.org/integration/mozilla-inbound/rev/66fb17ea8b049a40f14ff96e0ed1614ef1880207

Comment 14

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/66fb17ea8b04

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Oops! Thank you.

Comment 16

[Chris Mills [:cmills]]

I've added a note to the Fx65 rel notes to cover this:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/65#APIs

I've also removed the tiny mention of this event that I found in the main event ref landing page. 

So I think we can call this dev-doc-complete.