From b3a42e33a12c825998390f54de8bbe7f0ca4494b Mon Sep 17 00:00:00 2001 From: Josh Kelley Date: Mon, 27 Nov 2023 20:48:47 -0500 Subject: [PATCH 1/2] Fix for getLabelAndValue on null controller I encountered #11315 under the following circumstances: 1. Position the cursor over the chart area, such that it causes a tooltip to be shown. 2. Move the cursor out of the chart area, such that the tooltip remains visible. 3. Cause the chart contents to be changed, such that the dataset referenced by the active tooltip element is no longer valid. 4. Move the mouse again. This triggers an `inChartArea = false` event, so it reuses the previous, now invalid, active elements. This fixes #11315 under the circumstances for which I've reproduced it, but there may be others. --- src/plugins/plugin.tooltip.js | 3 ++- test/specs/plugin.tooltip.tests.js | 42 ++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/plugins/plugin.tooltip.js b/src/plugins/plugin.tooltip.js index 2e881417f78..c86467373ef 100644 --- a/src/plugins/plugin.tooltip.js +++ b/src/plugins/plugin.tooltip.js @@ -1182,7 +1182,8 @@ export class Tooltip extends Element { if (!inChartArea) { // Let user control the active elements outside chartArea. Eg. using Legend. - return lastActive; + // But make sure that active elements are still valid. + return lastActive.filter(i => this.chart.data.datasets[i.datasetIndex]); } // Find Active Elements for tooltips diff --git a/test/specs/plugin.tooltip.tests.js b/test/specs/plugin.tooltip.tests.js index da0563f6253..21c0c1bb23d 100644 --- a/test/specs/plugin.tooltip.tests.js +++ b/test/specs/plugin.tooltip.tests.js @@ -1667,6 +1667,48 @@ describe('Plugin.Tooltip', function() { }); }); + it('should tolerate datasets removed or added on events outside chartArea', async function() { + const dataset1 = { + label: 'Dataset 1', + data: [10, 20, 30], + }; + const dataset2 = { + label: 'Dataset 2', + data: [10, 25, 35], + }; + const chart = window.acquireChart({ + type: 'line', + data: { + datasets: [dataset1, dataset2], + labels: ['Point 1', 'Point 2', 'Point 3'] + }, + options: { + plugins: { + tooltip: { + mode: 'index', + intersect: false + } + } + } + }); + + var meta = chart.getDatasetMeta(0); + var point = meta.data[1]; + var expectedPoints = [jasmine.objectContaining({datasetIndex: 0, index: 1}), jasmine.objectContaining({datasetIndex: 1, index: 1})]; + + await jasmine.triggerMouseEvent(chart, 'mousemove', point); + await jasmine.triggerMouseEvent(chart, 'mousemove', {x: chart.chartArea.left - 5, y: point.y}); + + expect(chart.tooltip.getActiveElements()).toEqual(expectedPoints); + + chart.data.datasets = [dataset1]; + chart.update(); + + await jasmine.triggerMouseEvent(chart, 'mousemove', {x: 2, y: 1}); + + expect(chart.tooltip.getActiveElements()).toEqual([expectedPoints[0]]); + }); + describe('events', function() { it('should not be called on events not in plugin events array', async function() { var chart = window.acquireChart({ From 42577a000bb71d63ae41e4da04d0db32c02eaa62 Mon Sep 17 00:00:00 2001 From: Josh Kelley Date: Mon, 27 Nov 2023 21:14:38 -0500 Subject: [PATCH 2/2] Further fixes for elements added / changed This possibly fixes #11365. --- src/plugins/plugin.tooltip.js | 5 ++- test/specs/plugin.tooltip.tests.js | 51 +++++++++++++++++++++++++++--- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/src/plugins/plugin.tooltip.js b/src/plugins/plugin.tooltip.js index c86467373ef..05b5fb9470d 100644 --- a/src/plugins/plugin.tooltip.js +++ b/src/plugins/plugin.tooltip.js @@ -1183,7 +1183,10 @@ export class Tooltip extends Element { if (!inChartArea) { // Let user control the active elements outside chartArea. Eg. using Legend. // But make sure that active elements are still valid. - return lastActive.filter(i => this.chart.data.datasets[i.datasetIndex]); + return lastActive.filter(i => + this.chart.data.datasets[i.datasetIndex] && + this.chart.getDatasetMeta(i.datasetIndex).controller.getParsed(i.index) !== undefined + ); } // Find Active Elements for tooltips diff --git a/test/specs/plugin.tooltip.tests.js b/test/specs/plugin.tooltip.tests.js index 21c0c1bb23d..3314ce1385d 100644 --- a/test/specs/plugin.tooltip.tests.js +++ b/test/specs/plugin.tooltip.tests.js @@ -1667,7 +1667,7 @@ describe('Plugin.Tooltip', function() { }); }); - it('should tolerate datasets removed or added on events outside chartArea', async function() { + it('should tolerate datasets removed on events outside chartArea', async function() { const dataset1 = { label: 'Dataset 1', data: [10, 20, 30], @@ -1692,9 +1692,9 @@ describe('Plugin.Tooltip', function() { } }); - var meta = chart.getDatasetMeta(0); - var point = meta.data[1]; - var expectedPoints = [jasmine.objectContaining({datasetIndex: 0, index: 1}), jasmine.objectContaining({datasetIndex: 1, index: 1})]; + const meta = chart.getDatasetMeta(0); + const point = meta.data[1]; + const expectedPoints = [jasmine.objectContaining({datasetIndex: 0, index: 1}), jasmine.objectContaining({datasetIndex: 1, index: 1})]; await jasmine.triggerMouseEvent(chart, 'mousemove', point); await jasmine.triggerMouseEvent(chart, 'mousemove', {x: chart.chartArea.left - 5, y: point.y}); @@ -1709,6 +1709,49 @@ describe('Plugin.Tooltip', function() { expect(chart.tooltip.getActiveElements()).toEqual([expectedPoints[0]]); }); + it('should tolerate elements removed on events outside chartArea', async function() { + const dataset1 = { + label: 'Dataset 1', + data: [10, 20, 30], + }; + const dataset2 = { + label: 'Dataset 2', + data: [10, 25, 35], + }; + const chart = window.acquireChart({ + type: 'line', + data: { + datasets: [dataset1, dataset2], + labels: ['Point 1', 'Point 2', 'Point 3'] + }, + options: { + plugins: { + tooltip: { + mode: 'index', + intersect: false + } + } + } + }); + + const meta = chart.getDatasetMeta(0); + const point = meta.data[1]; + const expectedPoints = [jasmine.objectContaining({datasetIndex: 0, index: 1}), jasmine.objectContaining({datasetIndex: 1, index: 1})]; + + await jasmine.triggerMouseEvent(chart, 'mousemove', point); + await jasmine.triggerMouseEvent(chart, 'mousemove', {x: chart.chartArea.left - 5, y: point.y}); + + expect(chart.tooltip.getActiveElements()).toEqual(expectedPoints); + + dataset1.data = dataset1.data.slice(0, 1); + chart.data.datasets = [dataset1]; + chart.update(); + + await jasmine.triggerMouseEvent(chart, 'mousemove', {x: 2, y: 1}); + + expect(chart.tooltip.getActiveElements()).toEqual([]); + }); + describe('events', function() { it('should not be called on events not in plugin events array', async function() { var chart = window.acquireChart({