Skip to content

Commit 2381c36

Browse files
committed
fix(event): check hydration before firing event.
It is unlikely, but it can happen that an event is fired on a dehydrated view. Extra guard asserts the events fire only on hydrated views.
1 parent fb1b1da commit 2381c36

File tree

2 files changed

+54
-1
lines changed

2 files changed

+54
-1
lines changed

modules/core/src/compiler/view.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -381,10 +381,14 @@ export class ProtoView {
381381
MapWrapper.forEach(binder.events, (expr, eventName) => {
382382
DOM.on(element, eventName, (event) => {
383383
if (event.target === element) {
384+
// Most of the time the event will be fired only when the view is
385+
// in the live document. However, in a rare circumstance the
386+
// view might get dehydrated, in between the event queuing up and
387+
// firing.
384388
// TODO(rado): replace with
385389
// expr.eval(new ContextWithVariableBindings(view.context, {'$event': event}));
386390
// when eval with variable bindinds works.
387-
expr.eval(view.context);
391+
if (view.hydrated()) expr.eval(view.context);
388392
}
389393
});
390394
});

modules/core/test/compiler/view_spec.js

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,54 @@ export function main() {
403403
});
404404
});
405405

406+
describe('event handlers', () => {
407+
var view, ctx, called;
408+
409+
function createViewAndContext(protoView) {
410+
view = createView(protoView);
411+
ctx = view.context;
412+
called = 0;
413+
ctx.callMe = () => called += 1;
414+
}
415+
416+
function dispatchClick(el) {
417+
DOM.dispatchEvent(el, DOM.createMouseEvent('click'));
418+
}
419+
420+
function createProtoView() {
421+
var pv = new ProtoView(el('<div class="ng-binding"><div></div></div>'),
422+
new ProtoRecordRange());
423+
pv.bindElement(new TestProtoElementInjector(null, 0, []));
424+
pv.bindEvent('click', parser.parseBinding('callMe()', null));
425+
return pv;
426+
}
427+
428+
it('should fire on non-bubbling native events', () => {
429+
createViewAndContext(createProtoView());
430+
431+
dispatchClick(view.nodes[0]);
432+
433+
expect(called).toEqual(1);
434+
});
435+
436+
it('should not fire on a bubbled native events', () => {
437+
createViewAndContext(createProtoView());
438+
439+
dispatchClick(view.nodes[0].firstChild);
440+
441+
// This test passes trivially on webkit browsers due to
442+
// https://bugs.webkit.org/show_bug.cgi?id=122755
443+
expect(called).toEqual(0);
444+
});
445+
446+
it('should not throw if the view is dehydrated', () => {
447+
createViewAndContext(createProtoView());
448+
449+
view.dehydrate();
450+
dispatchClick(view.nodes[0]);
451+
});
452+
});
453+
406454
describe('react to record changes', () => {
407455
var view, cd, ctx;
408456

@@ -583,6 +631,7 @@ class MyEvaluationContext {
583631
foo:string;
584632
a;
585633
b;
634+
callMe;
586635
constructor() {
587636
this.foo = 'bar';
588637
};

0 commit comments

Comments
 (0)