From 1794a8e42a9b77efe3204c3bcf3f0dc563f58460 Mon Sep 17 00:00:00 2001
From: Kara Erickson <karakara@google.com>
Date: Tue, 16 Apr 2019 12:02:42 -0700
Subject: [PATCH] fix(ivy): coalesced listeners should preventDefault if any
 returns false (#29934)

We had a bug where event.preventDefault() was not always called if listeners
were coalesced. This is because we were overwriting the previous listener's
result every time we called the next listener, so listeners early in the chain
that returned false would be ignored and preventDefault would not be called.

This commit fixes that issue, so now preventDefault() is called if any listener
in a coalesced chain returns false. This brings us in line with View Engine
behavior.

PR Close #29934
---
 .../core/src/render3/instructions/listener.ts |  9 ++-
 .../core/test/acceptance/listener_spec.ts     | 57 ++++++++++++++++++-
 2 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/packages/core/src/render3/instructions/listener.ts b/packages/core/src/render3/instructions/listener.ts
index 3cb77b92c8..fb02629b04 100644
--- a/packages/core/src/render3/instructions/listener.ts
+++ b/packages/core/src/render3/instructions/listener.ts
@@ -202,9 +202,11 @@ function listenerInternal(
   }
 }
 
-function executeListenerWithErrorHandling(lView: LView, listenerFn: (e?: any) => any, e: any): any {
+function executeListenerWithErrorHandling(
+    lView: LView, listenerFn: (e?: any) => any, e: any): boolean {
   try {
-    return listenerFn(e);
+    // Only explicitly returning false from a listener should preventDefault
+    return listenerFn(e) !== false;
   } catch (error) {
     handleError(lView, error);
     return false;
@@ -242,7 +244,8 @@ function wrapListener(
     // their presence and invoke as needed.
     let nextListenerFn = (<any>wrapListenerIn_markDirtyAndPreventDefault).__ngNextListenerFn__;
     while (nextListenerFn) {
-      result = executeListenerWithErrorHandling(lView, nextListenerFn, e);
+      // We should prevent default if any of the listeners explicitly return false
+      result = executeListenerWithErrorHandling(lView, nextListenerFn, e) && result;
       nextListenerFn = (<any>nextListenerFn).__ngNextListenerFn__;
     }
 
diff --git a/packages/core/test/acceptance/listener_spec.ts b/packages/core/test/acceptance/listener_spec.ts
index 4ba782fcb2..b241a26383 100644
--- a/packages/core/test/acceptance/listener_spec.ts
+++ b/packages/core/test/acceptance/listener_spec.ts
@@ -42,6 +42,26 @@ describe('event listeners', () => {
       count() { this.counter++; }
     }
 
+    @Directive({selector: '[returns-false]'})
+    class ReturnsFalse {
+      counter = 0;
+      event !: Event;
+      handlerShouldReturn: boolean|undefined = undefined;
+
+      @HostListener('click', ['$event'])
+      count(e: Event) {
+        this.counter++;
+        this.event = e;
+
+        // stub preventDefault() to check whether it's called
+        Object.defineProperty(
+            this.event, 'preventDefault',
+            {value: jasmine.createSpy('preventDefault'), writable: true});
+
+        return this.handlerShouldReturn;
+      }
+    }
+
     onlyInIvy('ngDevMode.rendererAddEventListener counters are only available in ivy')
         .it('should coalesce multiple event listeners for the same event on the same element',
             () => {
@@ -119,5 +139,40 @@ describe('event listeners', () => {
       expect(noOfErrors).toBe(1);
       expect(buttonDebugEl.injector.get(LikesClicks).counter).toBe(1);
     });
+
+    it('should prevent default if any of the listeners returns false', () => {
+      @Component({
+        selector: 'test-cmpt',
+        template: `
+          <button returns-false likes-clicks></button>
+        `
+      })
+      class TestCmpt {
+      }
+
+      TestBed.configureTestingModule({declarations: [TestCmpt, ReturnsFalse, LikesClicks]});
+      const fixture = TestBed.createComponent(TestCmpt);
+      fixture.detectChanges();
+
+      const buttonDebugEl = fixture.debugElement.query(By.css('button'));
+      const likesClicksDir = buttonDebugEl.injector.get(LikesClicks);
+      const returnsFalseDir = buttonDebugEl.injector.get(ReturnsFalse);
+      expect(likesClicksDir.counter).toBe(0);
+      expect(returnsFalseDir.counter).toBe(0);
+
+      buttonDebugEl.nativeElement.click();
+      expect(likesClicksDir.counter).toBe(1);
+      expect(returnsFalseDir.counter).toBe(1);
+      expect(returnsFalseDir.event.preventDefault).not.toHaveBeenCalled();
+
+      returnsFalseDir.handlerShouldReturn = true;
+      buttonDebugEl.nativeElement.click();
+      expect(returnsFalseDir.event.preventDefault).not.toHaveBeenCalled();
+
+      returnsFalseDir.handlerShouldReturn = false;
+      buttonDebugEl.nativeElement.click();
+      expect(returnsFalseDir.event.preventDefault).toHaveBeenCalled();
+    });
+
   });
-});
\ No newline at end of file
+});