Closed Bug 1288640 Opened 7 years ago Closed 5 years ago

Stop dispatching "text" event in web contents

Categories

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

Product:
Core
Component:
DOM: Events
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla65
Tracking Flags:
Tracking Status
firefox50 --- affected
firefox65 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

(Keywords: dev-doc-complete, inputmethod, site-compat)

Attachments

(1 file)

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?
Flags: needinfo?(bugs)
bug 1506434 added the telemetry.
Flags: needinfo?(bugs)
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 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
https://hg.mozilla.org/mozilla-central/rev/8be20508c9d5
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox65: --- → fixed
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
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.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.