fix(core): prevent double invocation of load listener on img/iframe during event replay#68178
Draft
arturovt wants to merge 1 commit intoangular:mainfrom
Draft
fix(core): prevent double invocation of load listener on img/iframe during event replay#68178arturovt wants to merge 1 commit intoangular:mainfrom
arturovt wants to merge 1 commit intoangular:mainfrom
Conversation
Contributor
|
Hey @arturovt, per discussion offline, we couldn't land this as it is, as it's just a failing test, which would break CI. The only way we would accept this is if it also included the fix. |
Contributor
Author
makes sense, I've been working on a fix locally, I just need to sort things out, I would push the fix to the same branch. Will keep it in draft for now. |
…uring event replay When SSR with `withEventReplay()` is used in zoneless mode, Angular registers a native DOM listener on elements like `<img>` via `renderer.listen`, and also stashes the same listener in `__jsaction_fns` for replay. After hydration, `invokeListeners` calls the handler once (the replay), but the browser independently fires a native `load` event on the element as it finishes loading — triggering the listener a second time. The fix introduces a counter map on the element (keyed by event type) that `invokeListeners` increments before calling handlers. The native DOM listener then checks this map via `shouldSkipNativeListener` and skips itself if a replay already handled the event. The check is limited to elements that load external resources on their own (`img`, `iframe`, `script`, `link`, `object`, `embed`, `input`) — bubbling events like `click` on other elements are unaffected. `shouldSkipNativeListener` follows the `enableImpl` pattern: it defaults to `() => false` so terser/esbuild eliminate the branch entirely in apps that don't use event replay. `enableSkipNativeListenerImpl()` is called from the `withEventReplay()` environment initializer to swap in the real implementation. Fixes angular#59260
3d96dc1 to
6bcb31d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When SSR with
withEventReplay()is used in zoneless mode, Angular registers a native DOM listener on elements like<img>viarenderer.listen, and also stashes the same listener in__jsaction_fnsfor replay. After hydration,invokeListenerscalls the handler once (the replay), but the browser independently fires a nativeloadevent on the element as it finishes loading — triggering the listener a second time.The fix introduces a counter map on the element (keyed by event type) that
invokeListenersincrements before calling handlers. The native DOM listener then checks this map viashouldSkipNativeListenerand skips itself if a replay already handled the event.The check is limited to elements that load external resources on their own (
img,iframe,script,link,object,embed,input) — bubbling events likeclickon other elements are unaffected.shouldSkipNativeListenerfollows theenableImplpattern: it defaults to() => falseso terser/esbuild eliminate the branch entirely in apps that don't use event replay.enableSkipNativeListenerImpl()is called from thewithEventReplay()environment initializer to swap in the real implementation.Fixes #59260