Listen DOM events in each React container

[fcsonline]

Current React implementation is attaching events to document level.
This is breaking bubbling DOM behaviour not letting users to play
properly with methods like stopPropagation.

Also attaching events to containers instead of document integrates
better with other Javascript frameworks.

With this change we start attaching events at container level.

Fixes:
#2043
#4335

Related:
#2050
#1696

[ghost]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks!

[aweary]

How does this address the justifications mentioned in the documentation for ReactBrowserEventEmitter.listenTo for using a document level event listener?

   *
   * Firefox v8.01 (and possibly others) exhibited strange behavior when
   * mounting `onmousemove` events at some node that was not the document
   * element. The symptoms were that if your mouse is not moving over something
   * contained within that mount point (for example on the background) the
   * top-level listeners for `onmousemove` won't be called. However, if you
   * register the `mousemove` on the document object, then it will of course
   * catch all `mousemove`s. This along with iOS quirks, justifies restricting
   * top-level listeners to the document object only, at least for these
   * movement types of events and possibly all events.
   *

[ghost]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[fcsonline]

I didn't know about this behavior in those browsers, but I think that is interesting to evaluate if worth to have a weird behavior with modern browsers just to have a good behavior with old ones.

There are two approaches to deal with that:

• Detect those old browsers and enable a compatibility mode and attach events to document like now. If this is not the case attach events to specified node.
• Let developers choose if they want to support those old browsers with some flag in ReactDOM.render call.

I vote for the first approach, because we are not exposing an internal issue to the public API.

[aweary]

I didn't know about this behavior in those browsers, but I think that is interesting to evaluate if worth to have a weird behavior with modern browsers just to have a good behavior with old ones.

Behavior should be consistent across all supported browsers. Either we fully support a browser or we don't, there's no concept of "partially" supported browsers. This is one of the main reasons React does so many normalizations.

Detect those old browsers and enable a compatibility mode and attach events to document like now. If this is not the case attach events to specified node.

This feels like a hacky solution. As more and more issues are found relating to container-level event listeners this list of checks would just get bigger and bigger.

Let developers choose if they want to support those old browsers with some flag in ReactDOM.render call.

Increasing the API surface area should be considered carefully and done only in clearly advantageous situations. You don't see React's API being extended very often and there's a good reason for that. It also feels wrong to start introducing user-configurable options related to browser support.

I think that the issue this supposed to resolve is definitely an issue that should be addressed, but it will llikely take a more complicated implementation than this.

[fcsonline]

In #2050 we were talking about how this bug is breaking standard DOM event propagation. When you are integrating React with other javascript libraries it's impossible to stop the propagation of some events without hitting other bound libraries.

To understand the impact of this issue please review the following example:

http://jsbin.com/rafitiq/edit?html,js,output

Which are other approaches we have to address this issue?

[aweary]

#2050 was an attempt at fixing the bug you're describing, but it was closed as it was decided not to go forward with it. If this feature was approved by the React team it would need to be implemented in a safer, backwards compatible way (#2050 is probably a good reference)

I totally agree that this behavior is not ideal, I'm just saying that the fix is going to require a more robust implementation.

[gaearon]

Closing as stale.

[gaearon]

This was done in 17.
https://reactjs.org/blog/2020/08/10/react-v17-rc.html