From f6aa4080953916d7df64a3a897f528ed7bc3094e Mon Sep 17 00:00:00 2001
From: Benoit Socias <bso@odoo.com>
Date: Tue, 8 Aug 2023 13:24:48 +0000
Subject: [PATCH] [IMP] web: further document throttlingForAnimation

This is a followup on [1].

In Chrome the event's currentTarget is cleared after events such as
"scroll" are handled. For asynchronously called methods to be able to
access it, the current value of currentTarget needs to be kept.

To help developers that might stumble on this issue when using
`throttleForAnimation`, this commit emphasizes the fact that usage of
that function is not limited to event handlers, and it adds a test case
that illustrates a solution for tracking the lost scroll event target.

No scenario was identified in 15.0, but this could be used as an
alternative solution for [1].

[1]: https://github.com/odoo/odoo/commit/0ba601d2ef5c4e2f846818e78dcd23966d6f563d

task-3449843

X-original-commit: 6cc4cbf2b91d624368c489460a90795921985f6b
Part-of: odoo/odoo#161850
---
 addons/web/static/src/core/utils/timing.js    |  3 +
 .../static/tests/core/utils/timing_tests.js   | 63 +++++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/addons/web/static/src/core/utils/timing.js b/addons/web/static/src/core/utils/timing.js
index 1ef844b1e01..cb6b8043dbb 100644
--- a/addons/web/static/src/core/utils/timing.js
+++ b/addons/web/static/src/core/utils/timing.js
@@ -107,6 +107,9 @@ export function setRecurringAnimationFrame(callback) {
  * Creates a version of the function where only the last call between two
  * animation frames is executed before the browser's next repaint. This
  * effectively throttles the function to the display's refresh rate.
+ * Note that the throttled function can be any callback. It is not
+ * specifically an event handler, no assumption is made about its
+ * signature.
  * NB: The first call is always called immediately (leading edge).
  *
  * @template {Function} T
diff --git a/addons/web/static/tests/core/utils/timing_tests.js b/addons/web/static/tests/core/utils/timing_tests.js
index 33edbe241b1..72b2865103f 100644
--- a/addons/web/static/tests/core/utils/timing_tests.js
+++ b/addons/web/static/tests/core/utils/timing_tests.js
@@ -512,4 +512,67 @@ QUnit.module("utils", () => {
             });
         });
     });
+
+    QUnit.test("throttleForAnimationScrollEvent", async (assert) => {
+        assert.expect(9);
+        const { advanceFrame, execRegisteredAnimationFrames } = mockAnimationFrame();
+        let resolveThrottled;
+        let throttled = new Promise(resolve => resolveThrottled = resolve);
+        const throttledFn = throttleForAnimation((val, targetEl) => {
+            // In Chrome, the currentTarget of scroll events is lost after the
+            // event was handled, it is therefore null here.
+            // Because of this, if it is needed, it must be included in the
+            // callback signature.
+            const nodeName = val && val.currentTarget && val.currentTarget.nodeName;
+            const targetName = targetEl && targetEl.nodeName;
+            assert.step(`throttled function called with ${nodeName} in event, but ${targetName} in parameter`);
+            resolveThrottled();
+        });
+
+        const el = document.createElement("div");
+        el.style = "position: absolute; overflow: scroll; height: 100px; width: 100px;";
+        const childEl = document.createElement("div");
+        childEl.style = "height: 200px; width: 200px;";
+        let resolveScrolled;
+        let scrolled = new Promise(resolve => resolveScrolled = resolve);
+        el.appendChild(childEl);
+        el.addEventListener("scroll", (ev) => {
+            assert.step("before scroll");
+            throttledFn(ev, ev.currentTarget);
+            assert.step("after scroll");
+            resolveScrolled();
+        });
+        document.body.appendChild(el);
+        el.scrollBy(1, 1);
+        el.scrollBy(2, 2);
+        await scrolled;
+        await throttled;
+
+        assert.verifySteps([
+            "before scroll",
+            "throttled function called with DIV in event, but DIV in parameter",
+            "after scroll",
+        ], "scroll happened and direct first call to throttled function happened too");
+
+        throttled = new Promise(resolve => resolveThrottled = resolve);
+        scrolled = new Promise(resolve => resolveScrolled = resolve);
+        el.scrollBy(3, 3);
+        await scrolled;
+        assert.verifySteps([
+            "before scroll",
+            // Further call is delayed.
+            "after scroll",
+        ], "scroll happened but throttled function hasn't been called yet");
+        setTimeout(() => {
+            advanceFrame();
+            execRegisteredAnimationFrames();
+        });
+        await throttled;
+        assert.verifySteps(
+            ["throttled function called with null in event, but DIV in parameter"],
+            "currentTarget was not available in throttled function's event"
+        );
+        el.remove();
+    });
+
 });