diff --git a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js index 8f11847aac..5c2ff8c591 100644 --- a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js @@ -217,4 +217,165 @@ describe('ReactDOMEventListener', () => { expect(mouseOut.mock.calls[0][0]).toEqual(instance.getInner()); document.body.removeChild(container); }); + + // Regression test for https://github.com/facebook/react/pull/12877 + it('should not fire form events twice', () => { + const container = document.createElement('div'); + document.body.appendChild(container); + + const formRef = React.createRef(); + const inputRef = React.createRef(); + + const handleInvalid = jest.fn(); + const handleReset = jest.fn(); + const handleSubmit = jest.fn(); + ReactDOM.render( +
+ +
, + container, + ); + + inputRef.current.dispatchEvent( + new Event('invalid', { + // https://developer.mozilla.org/en-US/docs/Web/Events/invalid + bubbles: false, + }), + ); + expect(handleInvalid).toHaveBeenCalledTimes(1); + + formRef.current.dispatchEvent( + new Event('reset', { + // https://developer.mozilla.org/en-US/docs/Web/Events/reset + bubbles: true, + }), + ); + expect(handleReset).toHaveBeenCalledTimes(1); + + formRef.current.dispatchEvent( + new Event('submit', { + // https://developer.mozilla.org/en-US/docs/Web/Events/submit + bubbles: true, + }), + ); + expect(handleSubmit).toHaveBeenCalledTimes(1); + + formRef.current.dispatchEvent( + new Event('submit', { + // Might happen on older browsers. + bubbles: true, + }), + ); + expect(handleSubmit).toHaveBeenCalledTimes(2); // It already fired in this test. + + document.body.removeChild(container); + }); + + it('should dispatch loadstart only for media elements', () => { + const container = document.createElement('div'); + document.body.appendChild(container); + + const imgRef = React.createRef(); + const videoRef = React.createRef(); + + const handleImgLoadStart = jest.fn(); + const handleVideoLoadStart = jest.fn(); + ReactDOM.render( +
+ +
, + container, + ); + + // Note for debugging: loadstart currently doesn't fire in Chrome. + // https://bugs.chromium.org/p/chromium/issues/detail?id=458851 + imgRef.current.dispatchEvent( + new ProgressEvent('loadstart', { + bubbles: false, + }), + ); + // Historically, we happened to not support onLoadStart + // on , and this test documents that lack of support. + // If we decide to support it in the future, we should change + // this line to expect 1 call. Note that fixing this would + // be simple but would require attaching a handler to each + // . So far nobody asked us for it. + expect(handleImgLoadStart).toHaveBeenCalledTimes(0); + + videoRef.current.dispatchEvent( + new ProgressEvent('loadstart', { + bubbles: false, + }), + ); + expect(handleVideoLoadStart).toHaveBeenCalledTimes(1); + + document.body.removeChild(container); + }); + + it('should not attempt to listen to unnecessary events on the top level', () => { + const container = document.createElement('div'); + document.body.appendChild(container); + + const videoRef = React.createRef(); + const handleVideoPlay = jest.fn(); // We'll test this one. + const mediaEvents = { + onAbort() {}, + onCanPlay() {}, + onCanPlayThrough() {}, + onDurationChange() {}, + onEmptied() {}, + onEncrypted() {}, + onEnded() {}, + onError() {}, + onLoadedData() {}, + onLoadedMetadata() {}, + onLoadStart() {}, + onPause() {}, + onPlay() {}, + onPlaying() {}, + onProgress() {}, + onRateChange() {}, + onSeeked() {}, + onSeeking() {}, + onStalled() {}, + onSuspend() {}, + onTimeUpdate() {}, + onVolumeChange() {}, + onWaiting() {}, + }; + + const originalAddEventListener = document.addEventListener; + document.addEventListener = function(type) { + throw new Error( + `Did not expect to add a top-level listener for the "${type}" event.`, + ); + }; + + try { + // We expect that mounting this tree will + // *not* attach handlers for any top-level events. + ReactDOM.render( +
+
, + container, + ); + + // Also verify dispatching one of them works + videoRef.current.dispatchEvent( + new Event('play', { + bubbles: false, + }), + ); + expect(handleVideoPlay).toHaveBeenCalledTimes(1); + } finally { + document.addEventListener = originalAddEventListener; + document.body.removeChild(container); + } + }); }); diff --git a/packages/react-dom/src/events/DOMTopLevelEventTypes.js b/packages/react-dom/src/events/DOMTopLevelEventTypes.js index 298afec9c9..dc526a0ef0 100644 --- a/packages/react-dom/src/events/DOMTopLevelEventTypes.js +++ b/packages/react-dom/src/events/DOMTopLevelEventTypes.js @@ -148,6 +148,9 @@ export const TOP_VOLUME_CHANGE = unsafeCastStringToDOMTopLevelType( export const TOP_WAITING = unsafeCastStringToDOMTopLevelType('waiting'); export const TOP_WHEEL = unsafeCastStringToDOMTopLevelType('wheel'); +// List of events that need to be individually attached to media elements. +// Note that events in this list will *not* be listened to at the top level +// unless they're explicitly whitelisted in `ReactBrowserEventEmitter.listenTo`. export const mediaEventTypes = [ TOP_ABORT, TOP_CAN_PLAY, diff --git a/packages/react-dom/src/events/ReactBrowserEventEmitter.js b/packages/react-dom/src/events/ReactBrowserEventEmitter.js index d3291263ec..3dd2e452f5 100644 --- a/packages/react-dom/src/events/ReactBrowserEventEmitter.js +++ b/packages/react-dom/src/events/ReactBrowserEventEmitter.js @@ -13,9 +13,12 @@ import { TOP_CANCEL, TOP_CLOSE, TOP_FOCUS, + TOP_INVALID, TOP_RESET, TOP_SCROLL, TOP_SUBMIT, + getRawEventName, + mediaEventTypes, } from './DOMTopLevelEventTypes'; import { setEnabled, @@ -130,29 +133,40 @@ export function listenTo( for (let i = 0; i < dependencies.length; i++) { const dependency = dependencies[i]; if (!(isListening.hasOwnProperty(dependency) && isListening[dependency])) { - if (dependency === TOP_SCROLL) { - trapCapturedEvent(TOP_SCROLL, mountAt); - } else if (dependency === TOP_FOCUS || dependency === TOP_BLUR) { - trapCapturedEvent(TOP_FOCUS, mountAt); - trapCapturedEvent(TOP_BLUR, mountAt); - - // to make sure blur and focus event listeners are only attached once - isListening[TOP_BLUR] = true; - isListening[TOP_FOCUS] = true; - } else if (dependency === TOP_CANCEL) { - if (isEventSupported('cancel', true)) { - trapCapturedEvent(TOP_CANCEL, mountAt); - } - isListening[TOP_CANCEL] = true; - } else if (dependency === TOP_CLOSE) { - if (isEventSupported('close', true)) { - trapCapturedEvent(TOP_CLOSE, mountAt); - } - isListening[TOP_CLOSE] = true; - } else if (dependency !== TOP_SUBMIT && dependency !== TOP_RESET) { - trapBubbledEvent(dependency, mountAt); + switch (dependency) { + case TOP_SCROLL: + trapCapturedEvent(TOP_SCROLL, mountAt); + break; + case TOP_FOCUS: + case TOP_BLUR: + trapCapturedEvent(TOP_FOCUS, mountAt); + trapCapturedEvent(TOP_BLUR, mountAt); + // We set the flag for a single dependency later in this function, + // but this ensures we mark both as attached rather than just one. + isListening[TOP_BLUR] = true; + isListening[TOP_FOCUS] = true; + break; + case TOP_CANCEL: + case TOP_CLOSE: + if (isEventSupported(getRawEventName(dependency), true)) { + trapCapturedEvent(dependency, mountAt); + } + break; + case TOP_INVALID: + case TOP_SUBMIT: + case TOP_RESET: + // We listen to them on the target DOM elements. + // Some of them bubble so we don't want them to fire twice. + break; + default: + // By default, listen on the top level to all non-media events. + // Media events don't bubble so adding the listener wouldn't do anything. + const isMediaEvent = mediaEventTypes.indexOf(dependency) !== -1; + if (!isMediaEvent) { + trapBubbledEvent(dependency, mountAt); + } + break; } - isListening[dependency] = true; } }