fix(blaze): match delegated event selectors within delegation root - #513
Open
dupontbertrand wants to merge 1 commit into
Open
fix(blaze): match delegated event selectors within delegation root#513dupontbertrand wants to merge 1 commit into
dupontbertrand wants to merge 1 commit into
Conversation
The native (no-jQuery) backend decided delegation scope by comparing
$blaze_range.view.name strings between the delegation root and the node
where the selector stopped matching. Block helper views all share generic
names ('if', 'unless', 'each', ...), so a wrapper element placed inside a
parent's {{#if}} collided with the child template's own {{#if}} and the
event was silently dropped. When the clicked element was a direct child
of the delegation root, both names were even read off the same element,
dropping the event for any tagged wrapper.
Replace the name heuristic with the semantics jQuery actually applies:
evaluate the selector rooted at the delegation element (:scope prefix,
as $(elem).find(selector) does), then pick the closest matching
ancestor-or-self of the event target. This keeps the 'event map selector
scope' tests passing for the right reason and delivers events that the
heuristic wrongly discarded.
Fixes meteor#512
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
1 participant
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.
Fixes #512.
The native (no-jQuery) backend decided delegation scope by comparing
$blaze_range.view.namestrings between the delegation root and the node where the selector stopped matching. Block helper views share generic names ('if','unless', …), so a wrapper element inside a parent's{{#if}}collided with the child template's own{{#if}}and the event was silently dropped (#512 has the verified 8-case matrix; when the clicked element is a direct child of the delegation root, both names are even read off the same element, dropping the event for any tagged wrapper regardless of names).The heuristic was approximating jQuery's context-rooted selector matching (the "event map selector scope" tests: for
'div p', both elements must live inside the delegation root —$(elem).find(selector)semantics). This PR implements that semantics directly instead: prefix the selector with:scope, query from the delegation element, and pick the closest matching ancestor-or-self of the event target (root excluded, as jQuery does). The name comparison is gone.Notes:
expected "hit", actual ""); existing tests untouched — in particular the "event map selector scope" tests now pass for the right reason.test-in-console+puppeteerRunner): native 416/416, jQuery 420/420.querySelectorAllcost is equivalent to what jQuery's Sizzle did per delegated event on this same path; it only runs when jQuery is absent, and only for delegated (selector) handlers.:scopeis supported by every browser Meteor 3 targets, and the test suite itself already relies on it (querySelectorAll(':scope > *')).