Closed Bug 1288640 Opened 4 years ago Closed 2 years ago
Stop dispatching "text" event in web contents
Bug 1288640 - Make TextComposition not dispatch eCompositionChange events (DOM "text" event) in the default group of web content
47 bytes, text/x-phabricator-request
|Details | Review|
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?
(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.
(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?
bug 1506434 added the telemetry.
Priority: -- → P3
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.
Status: NEW → ASSIGNED
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.
Pushed by email@example.com: 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
Posted site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2018/non-standard-text-event-will-no-longer-be-fired-during-ime-composition/
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
Oops! Thank you.
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.
You need to log in before you can comment on or make changes to this bug.