From ce052e0992a8cb8493e14cabab42a2fd7d98572b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Thu, 20 Feb 2020 13:46:00 +0100 Subject: [PATCH] [FIX] transition: no crash if added/removed quickly This commit should fix crashes coming from transition code on nodes. This does not impact transitions on components. closes #637 closes #641 --- src/qweb/extensions.ts | 7 +++++++ tests/__snapshots__/animations.test.ts.snap | 21 +++++++++++++++++++ tests/animations.test.ts | 23 +++++++++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/src/qweb/extensions.ts b/src/qweb/extensions.ts index ad553b2c..abd22a97 100644 --- a/src/qweb/extensions.ts +++ b/src/qweb/extensions.ts @@ -183,6 +183,13 @@ function toMs(s: string): number { } function whenTransitionEnd(elm: HTMLElement, cb) { + if (!elm.parentNode) { + // if we get here, this means that the element was removed for some other + // reasons, and in that case, we don't want to work on animation since nothing + // will be displayed anyway. + return; + } + const styles = window.getComputedStyle(elm); const delays: Array = (styles.transitionDelay || "").split(", "); const durations: Array = (styles.transitionDuration || "").split(", "); diff --git a/tests/__snapshots__/animations.test.ts.snap b/tests/__snapshots__/animations.test.ts.snap index 558c60e3..821a1eff 100644 --- a/tests/__snapshots__/animations.test.ts.snap +++ b/tests/__snapshots__/animations.test.ts.snap @@ -177,3 +177,24 @@ exports[`animations t-transition, on a simple node (insert) 1`] = ` return vn1; }" `; + +exports[`animations t-transition, on a simple node, not in the DOM 1`] = ` +"function anonymous(context, extra +) { + // Template name: \\"test\\" + let utils = this.constructor.utils; + let h = this.h; + let c1 = [], p1 = {key:1}; + let vn1 = h('span', p1, c1); + p1.hook = { + insert: vn => { + utils.transitionInsert(vn, 'chimay'); + }, + remove: (vn, rm) => { + utils.transitionRemove(vn, 'chimay', rm); + }, + }; + c1.push({text: \`blue\`}); + return vn1; +}" +`; diff --git a/tests/animations.test.ts b/tests/animations.test.ts index d8b4574c..ee7626d7 100644 --- a/tests/animations.test.ts +++ b/tests/animations.test.ts @@ -78,6 +78,7 @@ describe("animations", () => { def.resolve(); }); let node: HTMLElement = renderToDOM(qweb, "test"); + fixture.appendChild(node); expect(node.className).toBe("chimay-enter chimay-enter-active"); await def; // wait for the mocked repaint to be done @@ -85,6 +86,27 @@ describe("animations", () => { expect(node.className).toBe(""); }); + test("t-transition, on a simple node, not in the DOM", async () => { + expect.assertions(5); + qweb.addTemplate("test", `blue`); + + let def = makeDeferred(); + patchNextFrame(cb => { + expect(node.className).toBe("chimay-enter chimay-enter-active"); + cb(); + expect(node.className).toBe("chimay-enter-active chimay-enter-to"); + def.resolve(); + }); + let node: HTMLElement = renderToDOM(qweb, "test"); + + expect(node.className).toBe("chimay-enter chimay-enter-active"); + await def; // wait for the mocked repaint to be done + node.dispatchEvent(new Event("transitionend")); + // we check here that the css classes have not been removed, since the + // element is not in the dom, we actually do not want to do anything. + expect(node.className).toBe("chimay-enter-active chimay-enter-to"); + }); + test("t-transition with no delay/duration", async () => { expect.assertions(4); qweb.addTemplate("test", `blue`); @@ -97,6 +119,7 @@ describe("animations", () => { def.resolve(); }); let node: HTMLElement = renderToDOM(qweb, "test"); + fixture.appendChild(node); expect(node.className).toBe("jupiler-enter jupiler-enter-active"); await def; });