Stop dispatching "text" event in web contents

RESOLVED FIXED in Firefox 65

Status

()

defect
P3
normal
RESOLVED FIXED
3 years ago
5 months ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

({dev-doc-complete, inputmethod, site-compat})

Trunk
mozilla65
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 affected, firefox65 fixed)

Details

Attachments

(1 attachment)

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?
Flags: needinfo?(bugs)
bug 1506434 added the telemetry.
Flags: needinfo?(bugs)
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.

Comment 10

6 months ago
Pushed by masayuki@d-toybox.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

Comment 11

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8be20508c9d5
Status: ASSIGNED → RESOLVED
Last Resolved: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
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
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.