diff --git a/src/traces/scattergl/index.js b/src/traces/scattergl/index.js index 8831fdb6bdb..63a0a03dcbf 100644 --- a/src/traces/scattergl/index.js +++ b/src/traces/scattergl/index.js @@ -395,18 +395,17 @@ function plot(gd, subplot, cdata) { scene.line2d.update(scene.lineOptions); scene.lineOptions = scene.lineOptions.map(function(lineOptions) { if(lineOptions && lineOptions.positions) { - var pos = [], srcPos = lineOptions.positions; + var srcPos = lineOptions.positions; var firstptdef = 0; - while(isNaN(srcPos[firstptdef]) || isNaN(srcPos[firstptdef + 1])) { + while(firstptdef < srcPos.length && (isNaN(srcPos[firstptdef]) || isNaN(srcPos[firstptdef + 1]))) { firstptdef += 2; } var lastptdef = srcPos.length - 2; - while(isNaN(srcPos[lastptdef]) || isNaN(srcPos[lastptdef + 1])) { - lastptdef += -2; + while(lastptdef > firstptdef && (isNaN(srcPos[lastptdef]) || isNaN(srcPos[lastptdef + 1]))) { + lastptdef -= 2; } - pos = pos.concat(srcPos.slice(firstptdef, lastptdef + 2)); - lineOptions.positions = pos; + lineOptions.positions = srcPos.slice(firstptdef, lastptdef + 2); } return lineOptions; }); @@ -437,36 +436,38 @@ function plot(gd, subplot, cdata) { if(trace._nexttrace) fillData.push(i + 1); if(fillData.length) scene.fillOrder[i] = fillData; - var pos = [], srcPos = (lineOptions && lineOptions.positions) || stash.positions; + var pos = []; + var srcPos = (lineOptions && lineOptions.positions) || stash.positions; + var firstptdef, lastptdef; if(trace.fill === 'tozeroy') { - var firstpdef = 0; - while(isNaN(srcPos[firstpdef + 1])) { - firstpdef += 2; + firstptdef = 0; + while(firstptdef < srcPos.length && isNaN(srcPos[firstptdef + 1])) { + firstptdef += 2; } - var lastpdef = srcPos.length - 2; - while(isNaN(srcPos[lastpdef + 1])) { - lastpdef += -2; + lastptdef = srcPos.length - 2; + while(lastptdef > firstptdef && isNaN(srcPos[lastptdef + 1])) { + lastptdef -= 2; } - if(srcPos[firstpdef + 1] !== 0) { - pos = [ srcPos[firstpdef], 0 ]; + if(srcPos[firstptdef + 1] !== 0) { + pos = [srcPos[firstptdef], 0]; } - pos = pos.concat(srcPos.slice(firstpdef, lastpdef + 2)); - if(srcPos[lastpdef + 1] !== 0) { - pos = pos.concat([ srcPos[lastpdef], 0 ]); + pos = pos.concat(srcPos.slice(firstptdef, lastptdef + 2)); + if(srcPos[lastptdef + 1] !== 0) { + pos = pos.concat([srcPos[lastptdef], 0]); } } else if(trace.fill === 'tozerox') { - var firstptdef = 0; - while(isNaN(srcPos[firstptdef])) { + firstptdef = 0; + while(firstptdef < srcPos.length && isNaN(srcPos[firstptdef])) { firstptdef += 2; } - var lastptdef = srcPos.length - 2; - while(isNaN(srcPos[lastptdef])) { - lastptdef += -2; + lastptdef = srcPos.length - 2; + while(lastptdef > firstptdef && isNaN(srcPos[lastptdef])) { + lastptdef -= 2; } if(srcPos[firstptdef] !== 0) { - pos = [ 0, srcPos[firstptdef + 1] ]; + pos = [0, srcPos[firstptdef + 1]]; } pos = pos.concat(srcPos.slice(firstptdef, lastptdef + 2)); if(srcPos[lastptdef] !== 0) { diff --git a/test/jasmine/tests/config_test.js b/test/jasmine/tests/config_test.js index dd6ce559ca0..7054a849f88 100644 --- a/test/jasmine/tests/config_test.js +++ b/test/jasmine/tests/config_test.js @@ -608,14 +608,14 @@ describe('config argument', function() { gd = parent.childNodes[0]; } - it('should resize when the viewport width/height changes', function(done) { + it('@flaky should resize when the viewport width/height changes', function(done) { fillParent(1, 1); Plotly.plot(gd, data, {}, {responsive: true}) .then(testResponsive) .then(done); }); - it('should still be responsive if the plot is edited', function(done) { + it('@flaky should still be responsive if the plot is edited', function(done) { fillParent(1, 1); Plotly.plot(gd, data, {}, {responsive: true}) .then(function() {return Plotly.restyle(gd, 'y[0]', data[0].y[0] + 2);}) @@ -623,7 +623,7 @@ describe('config argument', function() { .then(done); }); - it('should still be responsive if the plot is purged and replotted', function(done) { + it('@flaky should still be responsive if the plot is purged and replotted', function(done) { fillParent(1, 1); Plotly.plot(gd, data, {}, {responsive: true}) .then(function() {return Plotly.newPlot(gd, data, {}, {responsive: true});}) @@ -631,7 +631,7 @@ describe('config argument', function() { .then(done); }); - it('should only have one resize handler when plotted more than once', function(done) { + it('@flaky should only have one resize handler when plotted more than once', function(done) { fillParent(1, 1); var cntWindowResize = 0; window.addEventListener('resize', function() {cntWindowResize++;}); @@ -650,7 +650,7 @@ describe('config argument', function() { .then(done); }); - it('should become responsive if configured as such via Plotly.react', function(done) { + it('@flaky should become responsive if configured as such via Plotly.react', function(done) { fillParent(1, 1); Plotly.plot(gd, data, {}, {responsive: false}) .then(function() {return Plotly.react(gd, data, {}, {responsive: true});}) @@ -658,7 +658,7 @@ describe('config argument', function() { .then(done); }); - it('should stop being responsive if configured as such via Plotly.react', function(done) { + it('@flaky should stop being responsive if configured as such via Plotly.react', function(done) { fillParent(1, 1); Plotly.plot(gd, data, {}, {responsive: true}) // Check initial size @@ -676,7 +676,7 @@ describe('config argument', function() { }); // Testing fancier CSS layouts - it('should resize horizontally in a flexbox when responsive: true', function(done) { + it('@flaky should resize horizontally in a flexbox when responsive: true', function(done) { parent.style.display = 'flex'; parent.style.flexDirection = 'row'; fillParent(1, 2, function() { @@ -688,7 +688,7 @@ describe('config argument', function() { .then(done); }); - it('should resize vertically in a flexbox when responsive: true', function(done) { + it('@flaky should resize vertically in a flexbox when responsive: true', function(done) { parent.style.display = 'flex'; parent.style.flexDirection = 'column'; fillParent(2, 1, function() { @@ -700,7 +700,7 @@ describe('config argument', function() { .then(done); }); - it('should resize in both direction in a grid when responsive: true', function(done) { + it('@flaky should resize in both direction in a grid when responsive: true', function(done) { var numCols = 2, numRows = 2; parent.style.display = 'grid'; parent.style.gridTemplateColumns = 'repeat(' + numCols + ', 1fr)'; @@ -712,7 +712,7 @@ describe('config argument', function() { .then(done); }); - it('should provide a fixed non-zero width/height when autosize/responsive: true and container\' size is zero', function(done) { + it('@flaky should provide a fixed non-zero width/height when autosize/responsive: true and container\' size is zero', function(done) { fillParent(1, 1, function() { this.style.display = 'inline-block'; this.style.width = null; @@ -740,7 +740,7 @@ describe('config argument', function() { // The following test is to guarantee we're not breaking the existing (although maybe not ideal) behaviour. // In a future version, one may prefer responsive/autosize:true winning over an explicit width/height when embedded in a webpage. - it('should use the explicitly provided width/height even if autosize/responsive:true', function(done) { + it('@flaky should use the explicitly provided width/height even if autosize/responsive:true', function(done) { fillParent(1, 1, function() { this.style.width = '1000px'; this.style.height = '500px'; diff --git a/test/jasmine/tests/gl2d_plot_interact_test.js b/test/jasmine/tests/gl2d_plot_interact_test.js index e80d9606352..342b7cbc280 100644 --- a/test/jasmine/tests/gl2d_plot_interact_test.js +++ b/test/jasmine/tests/gl2d_plot_interact_test.js @@ -1186,6 +1186,79 @@ describe('Test gl2d plots', function() { .catch(failTest) .then(done); }); + + it('@gl should not cause infinite loops when coordinate arrays start/end with NaN', function(done) { + function _assertPositions(msg, cont, exp) { + var pos = gd._fullLayout._plots.xy._scene[cont] + .map(function(opt) { return opt.positions; }); + expect(pos).toBeCloseTo2DArray(exp, 2, msg); + } + + Plotly.plot(gd, [{ + type: 'scattergl', + mode: 'lines', + x: [1, 2, 3], + y: [null, null, null] + }, { + type: 'scattergl', + mode: 'lines', + x: [1, 2, 3], + y: [1, 2, null] + }, { + type: 'scattergl', + mode: 'lines', + x: [null, 2, 3], + y: [1, 2, 3] + }, { + type: 'scattergl', + mode: 'lines', + x: [null, null, null], + y: [1, 2, 3] + }, { + }]) + .then(function() { + _assertPositions('base', 'lineOptions', [ + [], + [1, 1, 2, 2], + [2, 2, 3, 3], + [] + ]); + + return Plotly.restyle(gd, 'fill', 'tozerox'); + }) + .then(function() { + _assertPositions('tozerox', 'lineOptions', [ + [], + [1, 1, 2, 2], + [2, 2, 3, 3], + [] + ]); + _assertPositions('tozerox', 'fillOptions', [ + [0, undefined, 0, undefined], + [0, 1, 1, 1, 2, 2, 0, 2], + [0, 2, 2, 2, 3, 3, 0, 3], + [0, undefined, 0, undefined] + ]); + + return Plotly.restyle(gd, 'fill', 'tozeroy'); + }) + .then(function() { + _assertPositions('tozeroy', 'lineOptions', [ + [], + [1, 1, 2, 2], + [2, 2, 3, 3], + [] + ]); + _assertPositions('tozeroy', 'fillOptions', [ + [undefined, 0, undefined, 0], + [1, 0, 1, 1, 2, 2, 2, 0], + [2, 0, 2, 2, 3, 3, 3, 0], + [undefined, 0, undefined, 0] + ]); + }) + .catch(failTest) + .then(done); + }); }); describe('Test scattergl autorange:', function() {