Skip to content

fix: bubble chart crash on window resize with named series in manual config #1267

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
HardeepAsrani wants to merge 1 commit into
base: development
Choose a base branch
from
+11 −6
Open

fix: bubble chart crash on window resize with named series in manual config #1267

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions js/render-google.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,17 @@ var isResizeRequest = false;
// remember, some charts do not support annotations so they should not be included in this.
var no_annotation_charts = ['tabular', 'timeline', 'gauge', 'geo', 'bubble', 'candlestick'];
if ( undefined !== chart.settings && undefined !== chart.settings.series && undefined === chart.settings.series.length ) {
var chartSeries = [];
var chartSeriesValue = Object.values( chart.settings.series );
$.each( Object.keys( chart.settings.series ), function( index, element ) {
chartSeries[element] = chartSeriesValue[index];
} );
chart.settings.series = chartSeries;
var seriesKeys = Object.keys( chart.settings.series );
// Only convert when keys are numeric indices (PHP JSON-encoded array).
// String keys (e.g. named series from manual config) must be left as-is.
if ( seriesKeys.every( function( k ) { return ! isNaN( k ); } ) ) {
var chartSeries = [];
var chartSeriesValue = Object.values( chart.settings.series );
$.each( seriesKeys, function( index, element ) {
chartSeries[ element ] = chartSeriesValue[ index ];
Comment on lines +37 to +41
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The numeric-key detection uses ! isNaN( k ), but isNaN performs coercion and will treat values like '1.5', '1e2', '', or '00' as “numeric”. Some of these are not valid array indices in JS (e.g. '1.5' / '00' become non-index properties), which can recreate the same sparse/length=0 behavior this change is trying to prevent. Consider using a stricter check for non-negative integer indices (e.g. /^(0|[1-9]\d*)$/ or Number.isInteger( Number(k) ) && String( Number(k) ) === k) and optionally casting element to a number when assigning into chartSeries.

Suggested change
if ( seriesKeys.every( function( k ) { return ! isNaN( k ); } ) ) {
var chartSeries = [];
var chartSeriesValue = Object.values( chart.settings.series );
$.each( seriesKeys, function( index, element ) {
chartSeries[ element ] = chartSeriesValue[ index ];
if ( seriesKeys.every( function( k ) { return /^(0|[1-9]\d*)$/.test( k ); } ) ) {
var chartSeries = [];
var chartSeriesValue = Object.values( chart.settings.series );
$.each( seriesKeys, function( index, element ) {
chartSeries[ Number( element ) ] = chartSeriesValue[ index ];

Copilot uses AI. Check for mistakes.
} );
chart.settings.series = chartSeries;
}
Comment on lines 33 to +44
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change fixes a user-facing crash on resize; the repo has Playwright e2e coverage for chart creation/embedding, but there doesn’t appear to be a regression test for resizing a Google Bubble chart with a manual config that defines named (string-keyed) series. Adding an e2e test that renders such a chart and triggers a viewport resize would help prevent this from coming back.

Copilot uses AI. Check for mistakes.
}
if(id !== 'canvas' && typeof chart.series !== 'undefined' && typeof chart.settings.series !== 'undefined' && ! no_annotation_charts.includes(chart.type) ) {
hasAnnotation = chart.series.length - chart.settings.series.length > 1;
Expand Down
Loading