Repository navigation
Conversation
The native (no-jQuery) bindEventCapturer reused the bubbling delegation wrapper, which walks from event.target up to the closest ancestor matching the selector. Capturing is what Blaze uses for non-bubbling events such as mouseenter/mouseleave, which the browser dispatches to every element the pointer crosses. Walking up therefore fired a 'mouseleave .card' handler, with currentTarget set to .card, when the pointer merely left a child of .card (and 'mouseenter .card' when it entered one), so hover states built on these handlers flicker or drop. Match the jQuery branch of bindEventCapturer instead: call the handler only when event.target itself is one of $(elem).find(selector), still evaluating the selector rooted at the delegation element. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
|
dupontbertrand
left a comment
dupontbertrand
left a comment
There was a problem hiding this comment.
Thanks @drone1, this matches the jQuery branch of bindEventCapturer exactly: the handler runs only when event.target itself is in $(elem).find(selector), with currentTarget = event.target and this === elem.
Verified locally on Meteor 3.5.3 with the full Blaze suite (test-in-console in headless Chromium; native mode uses a copy of test-in-console that doesn't pull in jquery, checked through Blaze._DOMBackend._hasJQuery):
| native | jQuery | |
|---|---|---|
release-3.1.0 (beta.0) |
420/420 | 424/424 |
| this PR | 421/421 | 425/425 |
this PR's test against the beta's dombackend.js |
the new test fails |
Same results on Meteor 3.6-rc.0.
One thing to carry over when the three PRs land: createCaptureWrapper sets currentTarget as an own property and never removes it, the leak #522 fixes in createWrapper. Whichever lands later should route it through #522's helper as well.
The CLA check is still pending on all three PRs.
|
@dupontbertrand the CLA check can only succeed, once @drone1 completes the CLA, linked in the comment |
signed. |
jankapunkt
left a comment
jankapunkt
left a comment
There was a problem hiding this comment.
I will merge this first as prposed, once the comments are converted to JSDoc function documenttion comments.
Please note I have changed the base from release-3.1.0 to fix/native-jquery-delegation which is a unaltered direct copy of release-3.1.0.
The reason is simple: all three PRs #518 #521 and #522 will be bundled into this branch while the branch itself will then finally merge into release-3.1.0 which will help us to keep a compact view of changed files of all three PRs. This is since the PRs have potential overlap and by retaining a narrow scope we might have it easier as human reviewers.
| // the pointer crosses; walking up from the target to the closest match would | ||
| // fire a 'mouseleave .card' handler when the pointer merely leaves a child | ||
| // of .card. | ||
| const createCaptureWrapper = (elem, selector, handler) => { |
There was a problem hiding this comment.
Please convert to a proper JSDoc comment, including parameter documentation
| // element ($(elem).find(selector)): for 'div p', both the div and the p | ||
| // must live inside `elem`. A bare closest(selector) matches against the | ||
| // whole document, letting ancestors outside `elem` satisfy the selector. | ||
| const scopeSelector = (selector) => selector |
There was a problem hiding this comment.
Please convert to a proper JSDoc comment, including parameter documentation
This branch has not been deployed
4 participants
Fixes #517.
The native (no-jQuery)
bindEventCapturerreusedcreateWrapper, the bubbling delegation wrapper, which walks fromevent.targetup to the closest ancestor matching the selector. Blaze uses capturing for non-bubbling events such asmouseenter/mouseleave, which the browser dispatches separately to every element the pointer crosses. Walking up therefore ran a'mouseleave .card'handler, withcurrentTargetset to.card, when the pointer only left a child of.card(and'mouseenter .card'when it entered one), so hover states built on these handlers would flicker or drop. Found while migrating a production app (Meteor 3.5.1,blaze@3.1.0-beta.0) off jQuery.This PR gives the native branch its own
createCaptureWrapperwith the semantics of the jQuery branch ($(event.target).is($(elem).find(selector))):event.targetitself is one ofelem.querySelectorAll(scopedSelector). The:scoperooting from fix(blaze): match delegated event selectors within delegation root #513 is moved into a sharedscopeSelectorhelper and reused.currentTargetis set to the target, and the handler is called withthis === elem, as in the jQuery branch.HandlerRec's capturing handler returns nothing anyway.handler._meteorui_wrapper, sounbindEventCaptureris unchanged.Notes:
spacebars-tests/templating_tests.js. It dispatches non-bubblingmouseenter/mouseleaveon a child of.card, on an unrelated sibling and on the delegation root, and expects none of them to run the.cardhandlers. Dispatching on.carditself must run both. I watched it fail before the fix (expected ["mouseenter card","mouseleave card"], actual [... ×4]).blazeandspacebars-testslocally with the CI method (meteor test-packages --driver-package test-in-console, driven headless in Chromium). Native: 348/348. jQuery (--extra-packages=jquery): 352/352. The new test passes on both backends, which confirms the two backends now agree.querySelectorAllmatches what$elem.find(selector)does per event on the jQuery path. It only runs for captured event types, whichHandlerRecalready filters.🤖 Generated with Claude Code