Let Wicket.Event.remove remove a listener added for several event types - #1648
Merged
Merged
Conversation
Wicket.Event.add in wicket-ajax-jquery.js accepts several space separated event types, as jQuery#on does, but it recorded the listener in its registry under the type string exactly as given, e.g. 'input change'. Wicket.Event.remove splits its type on whitespace and looks the listener up per type, so it found nothing and unbound nothing: remove(el, 'input change', fn), remove(el, 'input', fn) and remove(el, 'input change') all left both events bound. Only remove(el), without any type, still worked. This is a regression from 10.x, where remove delegated to jQuery#off, which handles several types. The jQuery-free wicket-ajax.js was not affected, because its add already records each type separately. add now does the same, so removing a listener works for all of its types at once and for each one on its own. Wicket's own code always adds one type at a time, so only application scripts that add a listener for several types and remove it again see a difference: the listener is now removed. GitHub issue #1646 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1648 +/- ##
============================================
- Coverage 61.95% 61.94% -0.01%
- Complexity 11280 11281 +1
============================================
Files 1250 1250
Lines 48429 48429
Branches 6792 6792
============================================
- Hits 30002 30000 -2
- Misses 15729 15730 +1
- Partials 2698 2699 +1 🚀 New features to boost your workflow:
|
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
3 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.
Fixes #1646.
Wicket.Event.addinwicket-ajax-jquery.jsrecorded a listener under the whole type string, e.g.'input change'.Wicket.Event.removesplits the type on whitespace before looking it up, so it never found a listener added for several types and unbound nothing. This is a regression from 10.x, whereremovedelegated tojQuery#off.addnow records the listener under each type separately, aswicket-ajax.jsalready does. That makes removal work for all the types at once and for each type on its own. Wicket's own code always adds one type at a time, so only application scripts are affected.Tests: three QUnit tests in
wicket-core/src/test/js/event.js, run against both engines. They cover removing all of the types, removing just one of them, and removing by type without a handler. All three fail on master'swicket-ajax-jquery.jsand pass with the fix.mvn clean verify -Pjs-testis green.🤖 Generated with Claude Code