Avoid recursive event replay loops (#8738)

* chart._lastEvent = null while processing onHover

* Pass replay flag to external tooltip

* Add test for replay

* cc

diff --git a/src/core/core.controller.js b/src/core/core.controller.js
index 8a8e6984e4cf9042f1bfef023ccb2a91397fb33c..a0458616f744e9683f2eab3ebd5e16b1c83e2823 100644 (file)
--- a/src/core/core.controller.js
+++ b/src/core/core.controller.js
@@ -1075,8 +1075,7 @@ class Chart {
         */
   _handleEvent(e, replay) {
     const me = this;
-    const lastActive = me._active || [];
-    const options = me.options;
+    const {_active: lastActive = [], options} = me;
     const hoverOptions = options.hover;
 
     // If the event is replayed from `update`, we should evaluate with the final positions.
@@ -1096,14 +1095,16 @@ class Chart {
 
     let active = [];
     let changed = false;
+    let lastEvent = null;
 
     // Find Active Elements for hover and tooltips
-    if (e.type === 'mouseout') {
-      me._lastEvent = null;
-    } else {
+    if (e.type !== 'mouseout') {
       active = me.getElementsAtEventForMode(e, hoverOptions.mode, hoverOptions, useFinalPosition);
-      me._lastEvent = e.type === 'click' ? me._lastEvent : e;
+      lastEvent = e.type === 'click' ? me._lastEvent : e;
     }
+    // Set _lastEvent to null while we are processing the event handlers.
+    // This prevents recursion if the handler calls chart.update()
+    me._lastEvent = null;
 
     // Invoke onHover hook
     callCallback(options.onHover || hoverOptions.onHover, [e, active, me], me);
@@ -1120,6 +1121,8 @@ class Chart {
       me._updateHoverStyles(active, lastActive, replay);
     }
 
+    me._lastEvent = lastEvent;
+
     return changed;
   }
 }

diff --git a/src/plugins/plugin.tooltip.js b/src/plugins/plugin.tooltip.js
index 6a488f01c79ba983091ec5831e696f411614b1a7..7b59d94938fbc0e03d5613cd2fab2796764e1fc0 100644 (file)
--- a/src/plugins/plugin.tooltip.js
+++ b/src/plugins/plugin.tooltip.js
@@ -524,7 +524,7 @@ export class Tooltip extends Element {
     return tooltipItems;
   }
 
-  update(changed) {
+  update(changed, replay) {
     const me = this;
     const options = me.options.setContext(me.getContext());
     const active = me._active;
@@ -574,7 +574,7 @@ export class Tooltip extends Element {
     }
 
     if (changed && options.external) {
-      options.external.call(me, {chart: me._chart, tooltip: me});
+      options.external.call(me, {chart: me._chart, tooltip: me, replay});
     }
   }
 
@@ -1023,7 +1023,7 @@ export class Tooltip extends Element {
           y: e.y
         };
 
-        me.update(true);
+        me.update(true, replay);
       }
     }
 

diff --git a/test/fixtures/controller.doughnut/event-replay.js b/test/fixtures/controller.doughnut/event-replay.js
new file mode 100644 (file)
index 0000000..fc367c3
--- /dev/null
+++ b/test/fixtures/controller.doughnut/event-replay.js
@@ -0,0 +1,50 @@
+function drawMousePoint(ctx, center) {
+  ctx.beginPath();
+  ctx.arc(center.x, center.y, 8, 0, Math.PI * 2);
+  ctx.fillStyle = 'yellow';
+  ctx.fill();
+}
+
+const canvas = document.createElement('canvas');
+canvas.width = 512;
+canvas.height = 512;
+const ctx = canvas.getContext('2d');
+
+module.exports = {
+  config: {
+    type: 'pie',
+    data: {
+      datasets: [{
+        backgroundColor: ['red', 'green', 'blue'],
+        hoverBackgroundColor: 'black',
+        data: [1, 1, 1]
+      }]
+    }
+  },
+  options: {
+    canvas: {
+      width: 512,
+      height: 512
+    },
+    async run(chart) {
+      ctx.drawImage(chart.canvas, 0, 0, 256, 256);
+
+      const arc = chart.getDatasetMeta(0).data[0];
+      const center = arc.getCenterPoint();
+      await jasmine.triggerMouseEvent(chart, 'mousemove', arc);
+      drawMousePoint(chart.ctx, center);
+      ctx.drawImage(chart.canvas, 256, 0, 256, 256);
+
+      chart.toggleDataVisibility(0);
+      chart.update();
+      drawMousePoint(chart.ctx, center);
+      ctx.drawImage(chart.canvas, 0, 256, 256, 256);
+
+      await jasmine.triggerMouseEvent(chart, 'mouseout', arc);
+      ctx.drawImage(chart.canvas, 256, 256, 256, 256);
+
+      Chart.helpers.clearCanvas(chart.canvas);
+      chart.ctx.drawImage(canvas, 0, 0);
+    }
+  }
+};

diff --git a/test/fixtures/controller.doughnut/event-replay.png b/test/fixtures/controller.doughnut/event-replay.png
new file mode 100644 (file)
index 0000000..5d14360
Binary files /dev/null and b/test/fixtures/controller.doughnut/event-replay.png differ

diff --git a/test/specs/plugin.tooltip.tests.js b/test/specs/plugin.tooltip.tests.js
index 2a1fe22fea7c013142963c59b466cec2f6fff917..20ca9428b31fb7c1c11e5dc1cbcfbff317eeb320 100644 (file)
--- a/test/specs/plugin.tooltip.tests.js
+++ b/test/specs/plugin.tooltip.tests.js
@@ -917,7 +917,7 @@ describe('Plugin.Tooltip', function() {
 
     // First dispatch change event, should update tooltip
     await jasmine.triggerMouseEvent(chart, 'mousemove', firstPoint);
-    expect(tooltip.update).toHaveBeenCalledWith(true);
+    expect(tooltip.update).toHaveBeenCalledWith(true, undefined);
 
     // Reset calls
     tooltip.update.calls.reset();
@@ -980,7 +980,7 @@ describe('Plugin.Tooltip', function() {
 
     // First dispatch change event, should update tooltip
     await jasmine.triggerMouseEvent(chart, 'mousemove', firstPoint);
-    expect(tooltip.update).toHaveBeenCalledWith(true);
+    expect(tooltip.update).toHaveBeenCalledWith(true, undefined);
 
     // Reset calls
     tooltip.update.calls.reset();
@@ -988,7 +988,7 @@ describe('Plugin.Tooltip', function() {
     // Second dispatch change event (same event), should update tooltip
     // because position mode is 'nearest'
     await jasmine.triggerMouseEvent(chart, 'mousemove', secondPoint);
-    expect(tooltip.update).toHaveBeenCalledWith(true);
+    expect(tooltip.update).toHaveBeenCalledWith(true, undefined);
   });
 
   describe('positioners', function() {