Skip to content

Commit 5a3ac37

Browse files
authored
fix: Apply preventDefault only to filtered events (#22294) (#22298)
When preventDefault() or stopPropagation() is used with setFilter(), it now only prevents default behavior for events that match the filter. This allows for more granular control, e.g., preventing default only for space key while allowing tab key to function normally. The fix modifies ElementListenerMap to check for preventDefault and stopPropagation expressions and make them conditional when a filter is present.
1 parent 29df465 commit 5a3ac37

File tree

3 files changed

+204
-3
lines changed

3 files changed

+204
-3
lines changed

flow-server/src/main/java/com/vaadin/flow/dom/DomListenerRegistration.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,10 @@ default DomListenerRegistration addEventDataElement(String eventData) {
363363

364364
/**
365365
* Stops propagation of the event to upper level DOM elements.
366+
* <p>
367+
* When used with {@link #setFilter(String)}, stopPropagation will only be
368+
* called when the filter condition is met. For example, if you set a filter
369+
* for specific keys, stopPropagation will only apply to those keys.
366370
*
367371
* @see <a href=
368372
* "https://developer.mozilla.org/en-US/docs/Web/API/Event/stopPropagation">MDN
@@ -379,6 +383,10 @@ default public DomListenerRegistration stopPropagation() {
379383
* Tries to prevent the default behavior of the event in the browser, such
380384
* as shortcut action on key press or context menu on "right click". This
381385
* might not be possible for some events.
386+
* <p>
387+
* When used with {@link #setFilter(String)}, preventDefault will only be
388+
* called when the filter condition is met. For example, if you set a filter
389+
* for specific keys, preventDefault will only apply to those keys.
382390
*
383391
* @see <a href=
384392
* "https://developer.mozilla.org/en-US/docs/Web/API/Event/preventDefault">MDN

flow-server/src/main/java/com/vaadin/flow/internal/nodefeature/ElementListenerMap.java

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -352,12 +352,40 @@ private Map<String, ExpressionSettings> collectEventExpressions(
352352
Collection<DomEventListenerWrapper> wrappers = getWrappers(eventType);
353353

354354
for (DomEventListenerWrapper wrapper : wrappers) {
355+
String filter = wrapper.getFilter();
356+
357+
// Process event data expressions, handling preventDefault and
358+
// stopPropagation specially
355359
if (wrapper.eventDataExpressions != null) {
356-
wrapper.eventDataExpressions.forEach(ensureExpression::apply);
360+
for (String expression : wrapper.eventDataExpressions) {
361+
// Check for preventDefault and stopPropagation
362+
if ("event.preventDefault()".equals(expression)) {
363+
if (filter != null && !filter.isEmpty()) {
364+
// If there's a filter, make preventDefault
365+
// conditional
366+
ensureExpression.apply("(" + filter
367+
+ ") && event.preventDefault()");
368+
} else {
369+
// No filter, keep it as is
370+
ensureExpression.apply(expression);
371+
}
372+
} else if ("event.stopPropagation()".equals(expression)) {
373+
if (filter != null && !filter.isEmpty()) {
374+
// If there's a filter, make stopPropagation
375+
// conditional
376+
ensureExpression.apply("(" + filter
377+
+ ") && event.stopPropagation()");
378+
} else {
379+
// No filter, keep it as is
380+
ensureExpression.apply(expression);
381+
}
382+
} else {
383+
// Other expressions, add as is
384+
ensureExpression.apply(expression);
385+
}
386+
}
357387
}
358388

359-
String filter = wrapper.getFilter();
360-
361389
int timeout = wrapper.debounceTimeout;
362390
if (timeout > 0 && filter == null) {
363391
filter = ALWAYS_TRUE_FILTER;

flow-server/src/test/java/com/vaadin/flow/internal/nodefeature/ElementListenersTest.java

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,171 @@ public void eventDataKeyNotPresentNotFail() {
491491
Assert.assertEquals(1, eventCount.get());
492492
}
493493

494+
@Test
495+
public void testPreventDefaultWithFilter() {
496+
// Test that preventDefault only applies to filtered events (see issue
497+
// #22294)
498+
499+
// Create a listener with filter for space and enter keys
500+
DomListenerRegistration registration = ns.add("keydown", noOp);
501+
registration.setFilter("event.key === ' ' || event.key === 'Enter'");
502+
registration.preventDefault();
503+
504+
// Check that the event data includes preventDefault
505+
Set<String> expressions = getExpressions("keydown");
506+
507+
// The expressions should include:
508+
// 1. The filter expression for debouncing
509+
// 2. The conditional preventDefault expression
510+
Assert.assertTrue("Should have the filter expression", expressions
511+
.contains("event.key === ' ' || event.key === 'Enter'"));
512+
513+
// After the fix, preventDefault should be conditional on the filter
514+
Assert.assertTrue("Should have conditional preventDefault expression",
515+
expressions.contains(
516+
"(event.key === ' ' || event.key === 'Enter') && event.preventDefault()"));
517+
518+
// The unconditional preventDefault should NOT be present
519+
Assert.assertFalse("Should NOT have unconditional preventDefault",
520+
expressions.contains("event.preventDefault()"));
521+
}
522+
523+
@Test
524+
public void testPreventDefaultWithoutFilter() {
525+
// Test preventDefault without filter - should apply to all events
526+
DomListenerRegistration registration = ns.add("keydown", noOp);
527+
registration.preventDefault();
528+
529+
Set<String> expressions = getExpressions("keydown");
530+
531+
// Without a filter, preventDefault should apply to all events
532+
Assert.assertTrue("Should have preventDefault expression",
533+
expressions.contains("event.preventDefault()"));
534+
Assert.assertEquals("Should only have preventDefault expression", 1,
535+
expressions.size());
536+
}
537+
538+
@Test
539+
public void testPreventDefaultThenSetFilter() {
540+
// Test that preventDefault becomes conditional even when filter is set
541+
// after
542+
DomListenerRegistration registration = ns.add("keydown", noOp);
543+
registration.preventDefault();
544+
registration.setFilter("event.key === 'Escape'");
545+
546+
Set<String> expressions = getExpressions("keydown");
547+
548+
// Should have conditional preventDefault based on the filter
549+
Assert.assertTrue("Should have conditional preventDefault expression",
550+
expressions.contains(
551+
"(event.key === 'Escape') && event.preventDefault()"));
552+
553+
// The unconditional preventDefault should NOT be present
554+
Assert.assertFalse("Should NOT have unconditional preventDefault",
555+
expressions.contains("event.preventDefault()"));
556+
}
557+
558+
@Test
559+
public void testSetFilterThenPreventDefault() {
560+
// Test that preventDefault is conditional when filter is set before
561+
DomListenerRegistration registration = ns.add("keydown", noOp);
562+
registration.setFilter("event.key === 'Delete'");
563+
registration.preventDefault();
564+
565+
Set<String> expressions = getExpressions("keydown");
566+
567+
// Should have conditional preventDefault based on the filter
568+
Assert.assertTrue("Should have conditional preventDefault expression",
569+
expressions.contains(
570+
"(event.key === 'Delete') && event.preventDefault()"));
571+
572+
// The unconditional preventDefault should NOT be present
573+
Assert.assertFalse("Should NOT have unconditional preventDefault",
574+
expressions.contains("event.preventDefault()"));
575+
}
576+
577+
@Test
578+
public void testStopPropagationWithFilter() {
579+
// Test that stopPropagation only applies to filtered events
580+
581+
// Create a listener with filter for space and enter keys
582+
DomListenerRegistration registration = ns.add("keydown", noOp);
583+
registration.setFilter("event.key === ' ' || event.key === 'Enter'");
584+
registration.stopPropagation();
585+
586+
// Check that the event data includes stopPropagation
587+
Set<String> expressions = getExpressions("keydown");
588+
589+
// The expressions should include:
590+
// 1. The filter expression for debouncing
591+
// 2. The conditional stopPropagation expression
592+
Assert.assertTrue("Should have the filter expression", expressions
593+
.contains("event.key === ' ' || event.key === 'Enter'"));
594+
595+
// After the fix, stopPropagation should be conditional on the filter
596+
Assert.assertTrue("Should have conditional stopPropagation expression",
597+
expressions.contains(
598+
"(event.key === ' ' || event.key === 'Enter') && event.stopPropagation()"));
599+
600+
// The unconditional stopPropagation should NOT be present
601+
Assert.assertFalse("Should NOT have unconditional stopPropagation",
602+
expressions.contains("event.stopPropagation()"));
603+
}
604+
605+
@Test
606+
public void testStopPropagationWithoutFilter() {
607+
// Test stopPropagation without filter - should apply to all events
608+
DomListenerRegistration registration = ns.add("keydown", noOp);
609+
registration.stopPropagation();
610+
611+
Set<String> expressions = getExpressions("keydown");
612+
613+
// Without a filter, stopPropagation should apply to all events
614+
Assert.assertTrue("Should have stopPropagation expression",
615+
expressions.contains("event.stopPropagation()"));
616+
Assert.assertEquals("Should only have stopPropagation expression", 1,
617+
expressions.size());
618+
}
619+
620+
@Test
621+
public void testStopPropagationThenSetFilter() {
622+
// Test that stopPropagation becomes conditional even when filter is
623+
// set after
624+
DomListenerRegistration registration = ns.add("keydown", noOp);
625+
registration.stopPropagation();
626+
registration.setFilter("event.key === 'Escape'");
627+
628+
Set<String> expressions = getExpressions("keydown");
629+
630+
// Should have conditional stopPropagation based on the filter
631+
Assert.assertTrue("Should have conditional stopPropagation expression",
632+
expressions.contains(
633+
"(event.key === 'Escape') && event.stopPropagation()"));
634+
635+
// The unconditional stopPropagation should NOT be present
636+
Assert.assertFalse("Should NOT have unconditional stopPropagation",
637+
expressions.contains("event.stopPropagation()"));
638+
}
639+
640+
@Test
641+
public void testSetFilterThenStopPropagation() {
642+
// Test that stopPropagation is conditional when filter is set before
643+
DomListenerRegistration registration = ns.add("keydown", noOp);
644+
registration.setFilter("event.key === 'Delete'");
645+
registration.stopPropagation();
646+
647+
Set<String> expressions = getExpressions("keydown");
648+
649+
// Should have conditional stopPropagation based on the filter
650+
Assert.assertTrue("Should have conditional stopPropagation expression",
651+
expressions.contains(
652+
"(event.key === 'Delete') && event.stopPropagation()"));
653+
654+
// The unconditional stopPropagation should NOT be present
655+
Assert.assertFalse("Should NOT have unconditional stopPropagation",
656+
expressions.contains("event.stopPropagation()"));
657+
}
658+
494659
// Helper for accessing package private API from other tests
495660
public static Set<String> getExpressions(
496661
ElementListenerMap elementListenerMap, String eventName) {

0 commit comments

Comments
 (0)