fix: bubble chart crash on window resize with named series in manual config#1267
fix: bubble chart crash on window resize with named series in manual config#1267HardeepAsrani wants to merge 1 commit intodevelopmentfrom
Conversation
…config When a manual config defines series with string keys (e.g. named series), override() permanently mutates chart.settings.series to a plain object. On resize, the numeric-index conversion incorrectly processed those string keys, producing a sparse array with length=0 that caused Google Charts to crash with "can't access property color, b is null".
HardeepAsrani
added
the
pr-checklist-skip
pirate-bot
added
the
pr-checklist-complete
There was a problem hiding this comment.
Pull request overview
Fixes a Google Bubble chart crash triggered on window resize when manual chart configs define series with string keys (named series), by preventing incorrect object-to-array conversion of chart.settings.series.
Changes:
- Detects whether
chart.settings.serieskeys are numeric before converting it into an array. - Leaves string-keyed
seriesobjects untouched to avoid creating sparse/empty arrays that can crash Google Charts on resize.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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 ]; |
There was a problem hiding this comment.
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.
| 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 ]; |
| 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 ]; | ||
| } ); | ||
| chart.settings.series = chartSeries; | ||
| } |
There was a problem hiding this comment.
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.
Summary
When a manual config defines series with string keys (e.g. named series), override() permanently mutates chart.settings.series to a plain object. On resize, the numeric-index conversion incorrectly processed those string keys, producing a sparse array with length=0 that caused Google Charts to crash with "can't access property color, b is null".
Will affect visual aspect of the product
YES/NO
Screenshots
Test instructions
Check before Pull Request is ready:
Closes https://github.com/Codeinwp/visualizer-pro/issues/485#issue-2423310972.