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
This commit is contained in:
Kara Erickson 2019-04-16 12:02:42 -07:00 committed by Alex Rickabaugh
parent 86a3f90954
commit 1794a8e42a
2 changed files with 62 additions and 4 deletions

View File

@ -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__;
}

View File

@ -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();
});
});
});
});