From 341963e76d72db3464fa2dd75f9680171e278d2d Mon Sep 17 00:00:00 2001 From: LucaVazz Date: Thu, 22 Apr 2021 18:07:13 +0200 Subject: [PATCH 1/9] replace d3.mouse in geo hover to work around firefox bug (#5525) --- src/plots/geo/geo.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/plots/geo/geo.js b/src/plots/geo/geo.js index c5789f600f2..766fd4a92e6 100644 --- a/src/plots/geo/geo.js +++ b/src/plots/geo/geo.js @@ -506,7 +506,11 @@ proto.updateFx = function(fullLayout, geoLayout) { } bgRect.on('mousemove', function() { - var lonlat = _this.projection.invert(d3.mouse(this)); + var d3EventPosition = [ + d3.event.offsetX, + d3.event.offsetY, + ]; + var lonlat = _this.projection.invert(d3EventPosition); if(!lonlat || isNaN(lonlat[0]) || isNaN(lonlat[1])) { return dragElement.unhover(gd, d3.event); From c4370065e54a5fd13426255768c50eb3621c5fa6 Mon Sep 17 00:00:00 2001 From: LucaVazz Date: Thu, 22 Apr 2021 18:10:22 +0200 Subject: [PATCH 2/9] extend choropleth tests to include translate3d transform (#5525) --- test/jasmine/tests/choropleth_test.js | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/test/jasmine/tests/choropleth_test.js b/test/jasmine/tests/choropleth_test.js index c619b66158d..acd7b228e30 100644 --- a/test/jasmine/tests/choropleth_test.js +++ b/test/jasmine/tests/choropleth_test.js @@ -167,7 +167,6 @@ describe('Test choropleth hover:', function() { function run(hasCssTransform, pos, fig, content, style) { gd = createGraphDiv(); - var scale = 1; style = style || { bgcolor: 'rgb(68, 68, 68)', @@ -177,14 +176,8 @@ describe('Test choropleth hover:', function() { fontFamily: 'Arial' }; - return Plotly.newPlot(gd, fig) - .then(function() { - if(hasCssTransform) { - scale = 0.5; - transformPlot(gd, 'translate(-25%, -25%) scale(0.5)'); - } - - mouseEvent('mousemove', scale * pos[0], scale * pos[1]); + function assertHoverLabel(posX, posY) { + mouseEvent('mousemove', posX, posY); assertHoverLabelContent({ nums: content[0], name: content[1] @@ -193,6 +186,18 @@ describe('Test choropleth hover:', function() { d3Select('g.hovertext'), style ); + } + + return Plotly.newPlot(gd, fig) + .then(function() { + if(hasCssTransform) { + transformPlot(gd, 'translate3d(10px, 10px, 0) scale(1)'); + assertHoverLabel(pos[0] + 10, pos[1] + 10) + transformPlot(gd, 'translate(-25%, -25%) scale(0.5)'); + assertHoverLabel(0.5 * pos[0], 0.5 * pos[1]) + } else { + assertHoverLabel(pos[0], pos[1]) + } }); } From 7b3929aa624003bb76f68c828ffc5d2e0e0a4d54 Mon Sep 17 00:00:00 2001 From: LucaVazz Date: Fri, 23 Apr 2021 11:26:20 +0200 Subject: [PATCH 3/9] fix coding style in changes --- test/jasmine/tests/choropleth_test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/jasmine/tests/choropleth_test.js b/test/jasmine/tests/choropleth_test.js index acd7b228e30..08085aef8dc 100644 --- a/test/jasmine/tests/choropleth_test.js +++ b/test/jasmine/tests/choropleth_test.js @@ -192,11 +192,11 @@ describe('Test choropleth hover:', function() { .then(function() { if(hasCssTransform) { transformPlot(gd, 'translate3d(10px, 10px, 0) scale(1)'); - assertHoverLabel(pos[0] + 10, pos[1] + 10) + assertHoverLabel(pos[0] + 10, pos[1] + 10); transformPlot(gd, 'translate(-25%, -25%) scale(0.5)'); - assertHoverLabel(0.5 * pos[0], 0.5 * pos[1]) + assertHoverLabel(0.5 * pos[0], 0.5 * pos[1]); } else { - assertHoverLabel(pos[0], pos[1]) + assertHoverLabel(pos[0], pos[1]); } }); } From f0e8e9884c1add0a37806d62efe2c22986e86f80 Mon Sep 17 00:00:00 2001 From: LucaVazz Date: Mon, 3 May 2021 17:37:28 +0200 Subject: [PATCH 4/9] move mouse position getter to lib and work-around Firefox bug (#5525) --- src/lib/index.js | 37 +++++++++++++++++++++++++++++++++++++ src/plots/geo/geo.js | 6 +----- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/src/lib/index.js b/src/lib/index.js index 685adcb962b..49b0f6789c9 100644 --- a/src/lib/index.js +++ b/src/lib/index.js @@ -715,6 +715,18 @@ lib.isIOS = function() { return IS_IOS_REGEX.test(window.navigator.userAgent); }; +var FIREFOX_VERSION_REGEX = /Firefox\/(\d+)\.\d+/; +lib.getFirefoxVersion = function() { + var match = FIREFOX_VERSION_REGEX.exec(window.navigator.userAgent); + if(match && match.length === 2) { + var versionInt = parseInt(match[1]); + if(!Number.isNaN(versionInt)) { + return versionInt; + } + } + return null; +}; + lib.isD3Selection = function(obj) { return obj instanceof d3.selection; }; @@ -1263,3 +1275,28 @@ lib.join2 = function(arr, mainSeparator, lastSeparator) { } return arr.join(mainSeparator); }; + +/** + * Return the mouse position from the last event registered by D3. + * @returns An array with two numbers, representing the x and y coordinates of the mouse pointer + * at the event relative to the targeted node. + */ +lib.getPositionFromD3Event = function() { + var firefoxVersion = lib.getFirefoxVersion(); + + // see https://bugzilla.mozilla.org/show_bug.cgi?id=1684973 + var isProblematicFirefox = firefoxVersion && firefoxVersion < 86; + + if(isProblematicFirefox) { + // layerX and layerY are non-standard, so we only fallback to them when we have to: + return [ + d3.event.layerX, + d3.event.layerY + ]; + } else { + return [ + d3.event.offsetX, + d3.event.offsetY + ]; + } +}; diff --git a/src/plots/geo/geo.js b/src/plots/geo/geo.js index 766fd4a92e6..5ed53ca379f 100644 --- a/src/plots/geo/geo.js +++ b/src/plots/geo/geo.js @@ -506,11 +506,7 @@ proto.updateFx = function(fullLayout, geoLayout) { } bgRect.on('mousemove', function() { - var d3EventPosition = [ - d3.event.offsetX, - d3.event.offsetY, - ]; - var lonlat = _this.projection.invert(d3EventPosition); + var lonlat = _this.projection.invert(Lib.getPositionFromD3Event()); if(!lonlat || isNaN(lonlat[0]) || isNaN(lonlat[1])) { return dragElement.unhover(gd, d3.event); From c80ce559463c7755f8db188bba399275e3b29495 Mon Sep 17 00:00:00 2001 From: LucaVazz Date: Mon, 3 May 2021 18:01:35 +0200 Subject: [PATCH 5/9] use global isNan for IE compatibility --- src/lib/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/index.js b/src/lib/index.js index 49b0f6789c9..1e2f62f767d 100644 --- a/src/lib/index.js +++ b/src/lib/index.js @@ -720,7 +720,7 @@ lib.getFirefoxVersion = function() { var match = FIREFOX_VERSION_REGEX.exec(window.navigator.userAgent); if(match && match.length === 2) { var versionInt = parseInt(match[1]); - if(!Number.isNaN(versionInt)) { + if(!isNaN(versionInt)) { return versionInt; } } From ff60f21f881e88395a603d0971efd8b213b198a0 Mon Sep 17 00:00:00 2001 From: LucaVazz Date: Wed, 5 May 2021 10:59:33 +0200 Subject: [PATCH 6/9] replace d3.mouse in parcats hover to work around firefox bug (#5607) --- src/traces/parcats/parcats.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/traces/parcats/parcats.js b/src/traces/parcats/parcats.js index 15f84b8bcb3..b2db22d26f7 100644 --- a/src/traces/parcats/parcats.js +++ b/src/traces/parcats/parcats.js @@ -399,7 +399,7 @@ function mouseoverPath(d) { // hoverinfo is a combination of 'count' and 'probability' // Mouse - var hoverX = d3.mouse(this)[0]; + var hoverX = Lib.getPositionFromD3Event()[0]; // Label var gd = d.parcatsViewModel.graphDiv; @@ -446,7 +446,7 @@ function mouseoverPath(d) { } var hovertext = hovertextParts.join('
'); - var mouseX = d3.mouse(gd)[0]; + var mouseX = Lib.getPositionFromD3Event()[0]; Fx.loneHover({ trace: trace, @@ -978,7 +978,7 @@ function mouseoverCategoryBand(bandViewModel) { // hoverinfo is not skip, so we at least style the bands and emit interaction events // Mouse - var mouseY = d3.mouse(this)[1]; + var mouseY = Lib.getPositionFromD3Event()[1]; if(mouseY < -1) { // Hover is above above the category rectangle (probably the dimension title text) return; @@ -1085,8 +1085,8 @@ function dragDimensionStart(d) { .each( /** @param {CategoryViewModel} catViewModel */ function(catViewModel) { - var catMouseX = d3.mouse(this)[0]; - var catMouseY = d3.mouse(this)[1]; + var catMouseX = Lib.getPositionFromD3Event()[0]; + var catMouseY = Lib.getPositionFromD3Event()[1]; if(-2 <= catMouseX && catMouseX <= catViewModel.width + 2 && From 56b3599dc0a9b1f70d2dc88bee8535517b3036c8 Mon Sep 17 00:00:00 2001 From: LucaVazz Date: Fri, 21 May 2021 11:38:58 +0200 Subject: [PATCH 7/9] Revert "replace d3.mouse in parcats hover to work around firefox bug (#5607)" This reverts commit ff60f21f881e88395a603d0971efd8b213b198a0. --- src/traces/parcats/parcats.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/traces/parcats/parcats.js b/src/traces/parcats/parcats.js index b2db22d26f7..15f84b8bcb3 100644 --- a/src/traces/parcats/parcats.js +++ b/src/traces/parcats/parcats.js @@ -399,7 +399,7 @@ function mouseoverPath(d) { // hoverinfo is a combination of 'count' and 'probability' // Mouse - var hoverX = Lib.getPositionFromD3Event()[0]; + var hoverX = d3.mouse(this)[0]; // Label var gd = d.parcatsViewModel.graphDiv; @@ -446,7 +446,7 @@ function mouseoverPath(d) { } var hovertext = hovertextParts.join('
'); - var mouseX = Lib.getPositionFromD3Event()[0]; + var mouseX = d3.mouse(gd)[0]; Fx.loneHover({ trace: trace, @@ -978,7 +978,7 @@ function mouseoverCategoryBand(bandViewModel) { // hoverinfo is not skip, so we at least style the bands and emit interaction events // Mouse - var mouseY = Lib.getPositionFromD3Event()[1]; + var mouseY = d3.mouse(this)[1]; if(mouseY < -1) { // Hover is above above the category rectangle (probably the dimension title text) return; @@ -1085,8 +1085,8 @@ function dragDimensionStart(d) { .each( /** @param {CategoryViewModel} catViewModel */ function(catViewModel) { - var catMouseX = Lib.getPositionFromD3Event()[0]; - var catMouseY = Lib.getPositionFromD3Event()[1]; + var catMouseX = d3.mouse(this)[0]; + var catMouseY = d3.mouse(this)[1]; if(-2 <= catMouseX && catMouseX <= catViewModel.width + 2 && From 344fb915df761718d5f9aac1f481c4fcbf66ac0a Mon Sep 17 00:00:00 2001 From: LucaVazz Date: Wed, 26 May 2021 14:46:52 +0200 Subject: [PATCH 8/9] adapt to review comments (#5525) --- src/lib/index.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/lib/index.js b/src/lib/index.js index e2db54193ea..0cbb71bf129 100644 --- a/src/lib/index.js +++ b/src/lib/index.js @@ -1280,17 +1280,16 @@ lib.bigFont = function(size) { return Math.round(1.2 * size); }; +var firefoxVersion = lib.getFirefoxVersion(); +// see https://bugzilla.mozilla.org/show_bug.cgi?id=1684973 +var isProblematicFirefox = firefoxVersion !== null && firefoxVersion < 86; + /** * Return the mouse position from the last event registered by D3. * @returns An array with two numbers, representing the x and y coordinates of the mouse pointer * at the event relative to the targeted node. */ lib.getPositionFromD3Event = function() { - var firefoxVersion = lib.getFirefoxVersion(); - - // see https://bugzilla.mozilla.org/show_bug.cgi?id=1684973 - var isProblematicFirefox = firefoxVersion && firefoxVersion < 86; - if(isProblematicFirefox) { // layerX and layerY are non-standard, so we only fallback to them when we have to: return [ From a9da031d22b9c77b152d4ccdb1688c548f88e0d4 Mon Sep 17 00:00:00 2001 From: LucaVazz Date: Wed, 26 May 2021 15:26:56 +0200 Subject: [PATCH 9/9] Revert "extend choropleth tests to include translate3d transform (#5525)" This reverts commit c4370065e54a5fd13426255768c50eb3621c5fa6. --- test/jasmine/tests/choropleth_test.js | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/test/jasmine/tests/choropleth_test.js b/test/jasmine/tests/choropleth_test.js index 08085aef8dc..c619b66158d 100644 --- a/test/jasmine/tests/choropleth_test.js +++ b/test/jasmine/tests/choropleth_test.js @@ -167,6 +167,7 @@ describe('Test choropleth hover:', function() { function run(hasCssTransform, pos, fig, content, style) { gd = createGraphDiv(); + var scale = 1; style = style || { bgcolor: 'rgb(68, 68, 68)', @@ -176,8 +177,14 @@ describe('Test choropleth hover:', function() { fontFamily: 'Arial' }; - function assertHoverLabel(posX, posY) { - mouseEvent('mousemove', posX, posY); + return Plotly.newPlot(gd, fig) + .then(function() { + if(hasCssTransform) { + scale = 0.5; + transformPlot(gd, 'translate(-25%, -25%) scale(0.5)'); + } + + mouseEvent('mousemove', scale * pos[0], scale * pos[1]); assertHoverLabelContent({ nums: content[0], name: content[1] @@ -186,18 +193,6 @@ describe('Test choropleth hover:', function() { d3Select('g.hovertext'), style ); - } - - return Plotly.newPlot(gd, fig) - .then(function() { - if(hasCssTransform) { - transformPlot(gd, 'translate3d(10px, 10px, 0) scale(1)'); - assertHoverLabel(pos[0] + 10, pos[1] + 10); - transformPlot(gd, 'translate(-25%, -25%) scale(0.5)'); - assertHoverLabel(0.5 * pos[0], 0.5 * pos[1]); - } else { - assertHoverLabel(pos[0], pos[1]); - } }); }