fix(ivy): preventDefault when listener returns false (#22529)

Closes #22495

PR Close #22529

packages/core/src/render3/instructions.ts

@@ -619,22 +619,24 @@ export function hostElement(rNode: RElement | null, def: ComponentDef<any>): LEl
  * and saves the subscription for later cleanup.
  *
  * @param eventName Name of the event
- * @param listener The function to be called when event emits
+ * @param listenerFn The function to be called when event emits
  * @param useCapture Whether or not to use capture in event listener.
  */
-export function listener(eventName: string, listener: EventListener, useCapture = false): void {
+export function listener(
+    eventName: string, listenerFn: (e?: any) => any, useCapture = false): void {
   ngDevMode && assertPreviousIsParent();
   const node = previousOrParentNode;
   const native = node.native as RElement;
-  const wrappedListener = wrapListenerWithDirtyLogic(currentView, listener);
 
   // In order to match current behavior, native DOM event listeners must be added for all
   // events (including outputs).
   const cleanupFns = cleanup || (cleanup = currentView.cleanup = []);
   if (isProceduralRenderer(renderer)) {
+    const wrappedListener = wrapListenerWithDirtyLogic(currentView, listenerFn);
     const cleanupFn = renderer.listen(native, eventName, wrappedListener);
     cleanupFns.push(cleanupFn, null);
   } else {
+    const wrappedListener = wrapListenerWithDirtyAndDefault(currentView, listenerFn);
     native.addEventListener(eventName, wrappedListener, useCapture);
     cleanupFns.push(eventName, native, wrappedListener, useCapture);
   }
@@ -649,7 +651,7 @@ export function listener(eventName: string, listener: EventListener, useCapture
   const outputs = tNode.outputs;
   let outputData: PropertyAliasValue|undefined;
   if (outputs && (outputData = outputs[eventName])) {
-    createOutput(outputData, listener);
+    createOutput(outputData, listenerFn);
   }
 }
 
@@ -1437,10 +1439,27 @@ export function markDirtyIfOnPush(node: LElementNode): void {
  * Wraps an event listener so its host view and its ancestor views will be marked dirty
  * whenever the event fires. Necessary to support OnPush components.
  */
-export function wrapListenerWithDirtyLogic(view: LView, listener: EventListener): EventListener {
+export function wrapListenerWithDirtyLogic(view: LView, listenerFn: (e?: any) => any): (e: Event) =>
+    any {
+  return function(e: any) {
+    markViewDirty(view);
+    return listenerFn(e);
+  };
+}
+
+/**
+ * Wraps an event listener so its host view and its ancestor views will be marked dirty
+ * whenever the event fires. Also wraps with preventDefault behavior.
+ */
+export function wrapListenerWithDirtyAndDefault(
+    view: LView, listenerFn: (e?: any) => any): EventListener {
   return function(e: Event) {
     markViewDirty(view);
-    listener(e);
+    if (listenerFn(e) === false) {
+      e.preventDefault();
+      // Necessary for legacy browsers that don't support preventDefault (e.g. IE)
+      e.returnValue = false;
+    }
   };
 }
 

packages/core/test/render3/compiler_canonical/elements_spec.ts

@@ -51,4 +51,42 @@ describe('elements', () => {
     expect(toHtml(renderComponent(MyComponent)))
         .toEqual('<div class="my-app" title="Hello">Hello <b>World</b>!</div>');
   });
+
+  it('should support listeners', () => {
+    type $ListenerComp$ = ListenerComp;
+
+    @Component({
+      selector: 'listener-comp',
+      template:
+          `<button (click)="onClick()" (keypress)="onPress($event); onPress2($event)">Click</button>`
+    })
+    class ListenerComp {
+      onClick() {}
+      onPress(e: Event) {}
+      onPress2(e: Event) {}
+
+      // NORMATIVE
+      static ngComponentDef = $r3$.ɵdefineComponent({
+        type: ListenerComp,
+        tag: 'listener-comp',
+        factory: function ListenerComp_Factory() { return new ListenerComp(); },
+        template: function ListenerComp_Template(ctx: $ListenerComp$, cm: $boolean$) {
+          if (cm) {
+            $r3$.ɵE(0, 'button');
+            $r3$.ɵL('click', function ListenerComp_click_Handler() { return ctx.onClick(); });
+            $r3$.ɵL('keypress', function ListenerComp_keypress_Handler($event: $any$) {
+              ctx.onPress($event);
+              return ctx.onPress2($event);
+            });
+            $r3$.ɵT(1, 'Click');
+            $r3$.ɵe();
+          }
+        }
+      });
+      // /NORMATIVE
+    }
+
+    const listenerComp = renderComponent(ListenerComp);
+    expect(toHtml(listenerComp)).toEqual('<button>Click</button>');
+  });
 });

packages/core/test/render3/listeners_spec.ts

@@ -9,6 +9,7 @@
 import {defineComponent, defineDirective} from '../../src/render3/index';
 import {container, containerRefreshEnd, containerRefreshStart, directiveRefresh, elementEnd, elementStart, embeddedViewEnd, embeddedViewStart, listener, text} from '../../src/render3/instructions';
 
+import {getRendererFactory2} from './imported_renderer2';
 import {containerEl, renderComponent, renderToHtml} from './render_util';
 
 
@@ -29,7 +30,7 @@ describe('event listeners', () => {
         if (cm) {
           elementStart(0, 'button');
           {
-            listener('click', function() { ctx.onClick(); });
+            listener('click', function() { return ctx.onClick(); });
             text(1, 'Click me');
           }
           elementEnd();
@@ -43,6 +44,39 @@ describe('event listeners', () => {
     });
   }
 
+  class PreventDefaultComp {
+    handlerReturnValue: any = true;
+    event: Event;
+
+    onClick(e: any) {
+      this.event = e;
+
+      // stub preventDefault() to check whether it's called
+      Object.defineProperty(
+          this.event, 'preventDefault',
+          {value: jasmine.createSpy('preventDefault'), writable: true});
+
+      return this.handlerReturnValue;
+    }
+
+    static ngComponentDef = defineComponent({
+      type: PreventDefaultComp,
+      tag: 'prevent-default-comp',
+      factory: () => new PreventDefaultComp(),
+      /** <button (click)="onClick($event)">Click</button> */
+      template: (ctx: PreventDefaultComp, cm: boolean) => {
+        if (cm) {
+          elementStart(0, 'button');
+          {
+            listener('click', function($event: any) { return ctx.onClick($event); });
+            text(1, 'Click');
+          }
+          elementEnd();
+        }
+      }
+    });
+  }
+
   beforeEach(() => { comps = []; });
 
   it('should call function on event emit', () => {
@@ -55,6 +89,38 @@ describe('event listeners', () => {
     expect(comp.counter).toEqual(2);
   });
 
+  it('should retain event handler return values using document', () => {
+    const preventDefaultComp = renderComponent(PreventDefaultComp);
+    const button = containerEl.querySelector('button') !;
+
+    button.click();
+    expect(preventDefaultComp.event !.preventDefault).not.toHaveBeenCalled();
+
+    preventDefaultComp.handlerReturnValue = undefined;
+    button.click();
+    expect(preventDefaultComp.event !.preventDefault).not.toHaveBeenCalled();
+
+    preventDefaultComp.handlerReturnValue = false;
+    button.click();
+    expect(preventDefaultComp.event !.preventDefault).toHaveBeenCalled();
+  });
+
+  it('should retain event handler return values with renderer2', () => {
+    const preventDefaultComp = renderComponent(PreventDefaultComp, getRendererFactory2(document));
+    const button = containerEl.querySelector('button') !;
+
+    button.click();
+    expect(preventDefaultComp.event !.preventDefault).not.toHaveBeenCalled();
+
+    preventDefaultComp.handlerReturnValue = undefined;
+    button.click();
+    expect(preventDefaultComp.event !.preventDefault).not.toHaveBeenCalled();
+
+    preventDefaultComp.handlerReturnValue = false;
+    button.click();
+    expect(preventDefaultComp.event !.preventDefault).toHaveBeenCalled();
+  });
+
   it('should call function chain on event emit', () => {
     /** <button (click)="onClick(); onClick2(); "> Click me </button> */
     function Template(ctx: any, cm: boolean) {
@@ -63,7 +129,7 @@ describe('event listeners', () => {
         {
           listener('click', function() {
             ctx.onClick();
-            ctx.onClick2();
+            return ctx.onClick2();
           });
           text(1, 'Click me');
         }
@@ -96,7 +162,7 @@ describe('event listeners', () => {
       if (cm) {
         elementStart(0, 'button');
         {
-          listener('click', function() { ctx.showing = !ctx.showing; });
+          listener('click', function() { return ctx.showing = !ctx.showing; });
           text(1, 'Click me');
         }
         elementEnd();
@@ -131,7 +197,7 @@ describe('event listeners', () => {
           if (embeddedViewStart(1)) {
             elementStart(0, 'button');
             {
-              listener('click', function() { ctx.onClick(); });
+              listener('click', function() { return ctx.onClick(); });
               text(1, 'Click me');
             }
             elementEnd();
@@ -170,7 +236,7 @@ describe('event listeners', () => {
         type: HostListenerDir,
         factory: function HostListenerDir_Factory() {
           const $dir$ = new HostListenerDir();
-          listener('click', function() { $dir$.onClick(); });
+          listener('click', function() { return $dir$.onClick(); });
           return $dir$;
         },
       });
@@ -222,7 +288,7 @@ describe('event listeners', () => {
               if (embeddedViewStart(0)) {
                 elementStart(0, 'button');
                 {
-                  listener('click', function() { ctx.onClick(); });
+                  listener('click', function() { return ctx.onClick(); });
                   text(1, 'Click');
                 }
                 elementEnd();
@@ -337,7 +403,7 @@ describe('event listeners', () => {
               if (embeddedViewStart(0)) {
                 elementStart(0, 'button');
                 {
-                  listener('click', function() { ctx.counter1++; });
+                  listener('click', function() { return ctx.counter1++; });
                   text(1, 'Click');
                 }
                 elementEnd();
@@ -352,7 +418,7 @@ describe('event listeners', () => {
               if (embeddedViewStart(0)) {
                 elementStart(0, 'button');
                 {
-                  listener('click', function() { ctx.counter2++; });
+                  listener('click', function() { return ctx.counter2++; });
                   text(1, 'Click');
                 }
                 elementEnd();

packages/core/test/render3/outputs_spec.ts

@@ -47,7 +47,7 @@ describe('outputs', () => {
       if (cm) {
         elementStart(0, ButtonToggle);
         {
-          listener('change', function() { ctx.onChange(); });
+          listener('change', function() { return ctx.onChange(); });
         }
         elementEnd();
       }
@@ -72,8 +72,8 @@ describe('outputs', () => {
       if (cm) {
         elementStart(0, ButtonToggle);
         {
-          listener('change', function() { ctx.onChange(); });
+          listener('change', function() { return ctx.onChange(); });
-          listener('reset', function() { ctx.onReset(); });
+          listener('reset', function() { return ctx.onReset(); });
         }
         elementEnd();
       }
@@ -99,7 +99,7 @@ describe('outputs', () => {
       if (cm) {
         elementStart(0, ButtonToggle);
         {
-          listener('change', function() { ctx.counter++; });
+          listener('change', function() { return ctx.counter++; });
         }
         elementEnd();
       }
@@ -135,7 +135,7 @@ describe('outputs', () => {
           if (embeddedViewStart(0)) {
             elementStart(0, ButtonToggle);
             {
-              listener('change', function() { ctx.onChange(); });
+              listener('change', function() { return ctx.onChange(); });
             }
             elementEnd();
           }
@@ -187,7 +187,7 @@ describe('outputs', () => {
               if (embeddedViewStart(0)) {
                 elementStart(0, ButtonToggle);
                 {
-                  listener('change', function() { ctx.onChange(); });
+                  listener('change', function() { return ctx.onChange(); });
                 }
                 elementEnd();
               }
@@ -249,13 +249,13 @@ describe('outputs', () => {
           if (embeddedViewStart(0)) {
             elementStart(0, 'button');
             {
-              listener('click', function() { ctx.onClick(); });
+              listener('click', function() { return ctx.onClick(); });
               text(1, 'Click me');
             }
             elementEnd();
             elementStart(2, ButtonToggle);
             {
-              listener('change', function() { ctx.onChange(); });
+              listener('change', function() { return ctx.onChange(); });
             }
             elementEnd();
             elementStart(4, DestroyComp);
@@ -311,7 +311,7 @@ describe('outputs', () => {
       if (cm) {
         elementStart(0, 'button', null, [MyButton]);
         {
-          listener('click', function() { ctx.onClick(); });
+          listener('click', function() { return ctx.onClick(); });
         }
         elementEnd();
       }
@@ -336,7 +336,7 @@ describe('outputs', () => {
       if (cm) {
         elementStart(0, ButtonToggle, null, [OtherDir]);
         {
-          listener('change', function() { ctx.onChange(); });
+          listener('change', function() { return ctx.onChange(); });
         }
         elementEnd();
       }
@@ -369,7 +369,7 @@ describe('outputs', () => {
       if (cm) {
         elementStart(0, ButtonToggle, null, [OtherDir]);
         {
-          listener('change', function() { ctx.onChange(); });
+          listener('change', function() { return ctx.onChange(); });
         }
         elementEnd();
       }
@@ -403,7 +403,7 @@ describe('outputs', () => {
       if (cm) {
         elementStart(0, 'button');
         {
-          listener('click', function() { ctx.onClick(); });
+          listener('click', function() { return ctx.onClick(); });
           text(1, 'Click me');
         }
         elementEnd();
@@ -415,7 +415,7 @@ describe('outputs', () => {
           if (embeddedViewStart(0)) {
             elementStart(0, ButtonToggle);
             {
-              listener('change', function() { ctx.onChange(); });
+              listener('change', function() { return ctx.onChange(); });
             }
             elementEnd();
           }
@@ -426,7 +426,7 @@ describe('outputs', () => {
           if (embeddedViewStart(1)) {
             elementStart(0, 'div', null, [OtherDir]);
             {
-              listener('change', function() { ctx.onChange(); });
+              listener('change', function() { return ctx.onChange(); });
             }
             elementEnd();
           }