From aa1622f3bc438663ad181e130cda0682545e48b4 Mon Sep 17 00:00:00 2001 From: archmoj Date: Wed, 26 May 2021 10:52:02 -0400 Subject: [PATCH 1/2] do not add quotes around url --- src/components/drawing/index.js | 2 +- test/jasmine/assets/custom_assertions.js | 4 ++-- test/jasmine/assets/get_bbox.js | 2 +- test/jasmine/tests/drawing_test.js | 8 ++++---- test/jasmine/tests/layout_images_test.js | 8 ++++---- test/jasmine/tests/plot_interact_test.js | 8 ++++---- test/jasmine/tests/shapes_test.js | 6 +++--- test/jasmine/tests/snapshot_test.js | 2 +- test/jasmine/tests/toimage_test.js | 8 ++++---- 9 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/components/drawing/index.js b/src/components/drawing/index.js index 2faf5f637ea..c5d04c8dbd7 100644 --- a/src/components/drawing/index.js +++ b/src/components/drawing/index.js @@ -1270,7 +1270,7 @@ function getFullUrl(localId, gd) { var context = gd._context; var baseUrl = context._exportedPlot ? '' : (context._baseUrl || ''); - return 'url(\'' + baseUrl + '#' + localId + '\')'; + return 'url(' + baseUrl + '#' + localId + ')'; } drawing.getTranslate = function(element) { diff --git a/test/jasmine/assets/custom_assertions.js b/test/jasmine/assets/custom_assertions.js index 93f41d7c058..d4df99b157d 100644 --- a/test/jasmine/assets/custom_assertions.js +++ b/test/jasmine/assets/custom_assertions.js @@ -226,8 +226,8 @@ exports.assertClip = function(sel, isClipped, size, msg) { var clipPath = d3Select(this).attr('clip-path'); if(isClipped) { - expect(String(clipPath).substr(0, 5)) - .toBe('url(\'', msg + ' clip path ' + '(item ' + i + ')'); + expect(String(clipPath).substr(0, 4)) + .toBe('url(', msg + ' clip path ' + '(item ' + i + ')'); } else { expect(clipPath) .toBe(null, msg + ' clip path ' + '(item ' + i + ')'); diff --git a/test/jasmine/assets/get_bbox.js b/test/jasmine/assets/get_bbox.js index e3ed22fe5c0..503f210eb6b 100644 --- a/test/jasmine/assets/get_bbox.js +++ b/test/jasmine/assets/get_bbox.js @@ -15,7 +15,7 @@ module.exports = function getBBox(element) { if(!clipPathAttr) return elementBBox; // only supports 'url(#)' at the moment - var clipPathId = clipPathAttr.substring(6, clipPathAttr.length - 2); + var clipPathId = clipPathAttr.substring(5, clipPathAttr.length - 1); var clipBBox = getClipBBox(clipPathId); return minBBox(elementBBox, clipBBox); diff --git a/test/jasmine/tests/drawing_test.js b/test/jasmine/tests/drawing_test.js index 40c39fe34dd..04406148c6c 100644 --- a/test/jasmine/tests/drawing_test.js +++ b/test/jasmine/tests/drawing_test.js @@ -25,11 +25,11 @@ describe('Drawing', function() { Drawing.setClipUrl(this.g, 'id1', {_context: {}}); - expect(this.g.attr('clip-path')).toEqual('url(\'#id1\')'); + expect(this.g.attr('clip-path')).toEqual('url(#id1)'); }); it('should unset the clip-path if arg is falsy', function() { - this.g.attr('clip-path', 'url(\'#id2\')'); + this.g.attr('clip-path', 'url(#id2)'); Drawing.setClipUrl(this.g, false); @@ -48,7 +48,7 @@ describe('Drawing', function() { Drawing.setClipUrl(this.g, 'id3', {_context: {_baseUrl: href}}); expect(this.g.attr('clip-path')) - .toEqual('url(\'' + href + '#id3\')'); + .toEqual('url(' + href + '#id3)'); base.remove(); }); @@ -64,7 +64,7 @@ describe('Drawing', function() { Drawing.setClipUrl(this.g, 'id4', {_context: {_baseUrl: href2}}); - var expected = 'url(\'' + href2 + '#id4\')'; + var expected = 'url(' + href2 + '#id4)'; expect(this.g.attr('clip-path')).toEqual(expected); diff --git a/test/jasmine/tests/layout_images_test.js b/test/jasmine/tests/layout_images_test.js index f0350fc09bf..625723860c7 100644 --- a/test/jasmine/tests/layout_images_test.js +++ b/test/jasmine/tests/layout_images_test.js @@ -524,14 +524,14 @@ describe('images log/linear axis changes', function() { // automatically when you change xref / yref, we leave it to the caller. // initial clip path should end in 'xy' to match xref/yref - expect(d3Select('image').attr('clip-path') || '').toMatch(/xy\'\)$/); + expect(d3Select('image').attr('clip-path') || '').toMatch(/xy\)$/); // linear to log Plotly.relayout(gd, {'images[0].yref': 'y2'}) .then(function() { expect(gd.layout.images[0].y).toBe(1); - expect(d3Select('image').attr('clip-path') || '').toMatch(/xy2\'\)$/); + expect(d3Select('image').attr('clip-path') || '').toMatch(/xy2\)$/); // log to paper return Plotly.relayout(gd, {'images[0].yref': 'paper'}); @@ -539,7 +539,7 @@ describe('images log/linear axis changes', function() { .then(function() { expect(gd.layout.images[0].y).toBe(1); - expect(d3Select('image').attr('clip-path') || '').toMatch(/x\'\)$/); + expect(d3Select('image').attr('clip-path') || '').toMatch(/x\)$/); // change to full paper-referenced, to make sure the clip path disappears return Plotly.relayout(gd, {'images[0].xref': 'paper'}); @@ -553,7 +553,7 @@ describe('images log/linear axis changes', function() { .then(function() { expect(gd.layout.images[0].y).toBe(1); - expect(d3Select('image').attr('clip-path') || '').toMatch(/^[^x]+y2\'\)$/); + expect(d3Select('image').attr('clip-path') || '').toMatch(/^[^x]+y2\)$/); // log to linear return Plotly.relayout(gd, {'images[0].yref': 'y'}); diff --git a/test/jasmine/tests/plot_interact_test.js b/test/jasmine/tests/plot_interact_test.js index f650e6c1d4b..86bedef3cf2 100644 --- a/test/jasmine/tests/plot_interact_test.js +++ b/test/jasmine/tests/plot_interact_test.js @@ -545,8 +545,8 @@ describe('plot svg clip paths', function() { d3SelectAll('[clip-path]').each(function() { var cp = d3Select(this).attr('clip-path'); - expect(cp.substring(0, 6)).toEqual('url(\'#'); - expect(cp.substring(cp.length - 2)).toEqual('\')'); + expect(cp.substring(0, 5)).toEqual('url(#'); + expect(cp.substring(cp.length - 1)).toEqual(')'); }); }) .then(done, done.fail); @@ -568,8 +568,8 @@ describe('plot svg clip paths', function() { d3SelectAll('[clip-path]').each(function() { var cp = d3Select(this).attr('clip-path'); - expect(cp.substring(0, 6 + href.length)).toEqual('url(\'' + href + '#'); - expect(cp.substring(cp.length - 2)).toEqual('\')'); + expect(cp.substring(0, 5 + href.length)).toEqual('url(' + href + '#'); + expect(cp.substring(cp.length - 1)).toEqual(')'); }); base.remove(); diff --git a/test/jasmine/tests/shapes_test.js b/test/jasmine/tests/shapes_test.js index 4f23c84db59..ed72199334d 100644 --- a/test/jasmine/tests/shapes_test.js +++ b/test/jasmine/tests/shapes_test.js @@ -494,7 +494,7 @@ describe('shapes axis reference changes', function() { } it('draws the right number of objects and updates clip-path correctly', function(done) { - expect(getShape(0).attr('clip-path') || '').toMatch(/x\'\)$/); + expect(getShape(0).attr('clip-path') || '').toMatch(/x\)$/); Plotly.relayout(gd, { 'shapes[0].xref': 'paper', @@ -511,7 +511,7 @@ describe('shapes axis reference changes', function() { }); }) .then(function() { - expect(getShape(0).attr('clip-path') || '').toMatch(/^[^x]+y2\'\)$/); + expect(getShape(0).attr('clip-path') || '').toMatch(/^[^x]+y2\)$/); return Plotly.relayout(gd, { 'shapes[0].xref': 'x', @@ -520,7 +520,7 @@ describe('shapes axis reference changes', function() { }); }) .then(function() { - expect(getShape(0).attr('clip-path') || '').toMatch(/xy2\'\)$/); + expect(getShape(0).attr('clip-path') || '').toMatch(/xy2\)$/); }) .then(done, done.fail); }); diff --git a/test/jasmine/tests/snapshot_test.js b/test/jasmine/tests/snapshot_test.js index 1cebfbadb9e..4335cc66a62 100644 --- a/test/jasmine/tests/snapshot_test.js +++ b/test/jasmine/tests/snapshot_test.js @@ -251,7 +251,7 @@ describe('Plotly.Snapshot', function() { describe('should handle quoted style properties', function() { function checkURL(actual, msg) { - // which is enough tot check that toSVG did its job right + // which is enough to check that toSVG did its job right expect((actual || '').substr(0, 6)).toBe('url(\"#', msg); } diff --git a/test/jasmine/tests/toimage_test.js b/test/jasmine/tests/toimage_test.js index cbaffb56efd..9c41a3ed461 100644 --- a/test/jasmine/tests/toimage_test.js +++ b/test/jasmine/tests/toimage_test.js @@ -239,10 +239,10 @@ describe('Plotly.toImage', function() { var clipPath = gSubplot.getAttribute('clip-path'); var len = clipPath.length; - var head = clipPath.substr(0, 5); - var tail = clipPath.substr(len - 8, len); - expect(head).toBe('url(\'', 'subplot clipPath head'); - expect(tail).toBe('xyplot\')', 'subplot clipPath tail'); + var head = clipPath.substr(0, 4); + var tail = clipPath.substr(len - 7, len); + expect(head).toBe('url(', 'subplot clipPath head'); + expect(tail).toBe('xyplot)', 'subplot clipPath tail'); var middle = clipPath.substr(4, 10); expect(middle.length).toBe(10, 'subplot clipPath uid length'); From 3a5d532644540fcbc3cb800f175e6d81a2873465 Mon Sep 17 00:00:00 2001 From: archmoj Date: Wed, 26 May 2021 12:17:03 -0400 Subject: [PATCH 2/2] keep quotes in the case of base url --- src/components/drawing/index.js | 4 +++- test/jasmine/tests/drawing_test.js | 6 +++--- test/jasmine/tests/plot_interact_test.js | 4 ++-- test/jasmine/tests/snapshot_test.js | 2 +- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/components/drawing/index.js b/src/components/drawing/index.js index c5d04c8dbd7..484539f20de 100644 --- a/src/components/drawing/index.js +++ b/src/components/drawing/index.js @@ -1270,7 +1270,9 @@ function getFullUrl(localId, gd) { var context = gd._context; var baseUrl = context._exportedPlot ? '' : (context._baseUrl || ''); - return 'url(' + baseUrl + '#' + localId + ')'; + return baseUrl ? + 'url(\'' + baseUrl + '#' + localId + '\')' : + 'url(#' + localId + ')'; } drawing.getTranslate = function(element) { diff --git a/test/jasmine/tests/drawing_test.js b/test/jasmine/tests/drawing_test.js index 04406148c6c..a8db5760313 100644 --- a/test/jasmine/tests/drawing_test.js +++ b/test/jasmine/tests/drawing_test.js @@ -29,7 +29,7 @@ describe('Drawing', function() { }); it('should unset the clip-path if arg is falsy', function() { - this.g.attr('clip-path', 'url(#id2)'); + this.g.attr('clip-path', 'url(\'#id2\')'); Drawing.setClipUrl(this.g, false); @@ -48,7 +48,7 @@ describe('Drawing', function() { Drawing.setClipUrl(this.g, 'id3', {_context: {_baseUrl: href}}); expect(this.g.attr('clip-path')) - .toEqual('url(' + href + '#id3)'); + .toEqual('url(\'' + href + '#id3\')'); base.remove(); }); @@ -64,7 +64,7 @@ describe('Drawing', function() { Drawing.setClipUrl(this.g, 'id4', {_context: {_baseUrl: href2}}); - var expected = 'url(' + href2 + '#id4)'; + var expected = 'url(\'' + href2 + '#id4\')'; expect(this.g.attr('clip-path')).toEqual(expected); diff --git a/test/jasmine/tests/plot_interact_test.js b/test/jasmine/tests/plot_interact_test.js index 86bedef3cf2..a4810a791eb 100644 --- a/test/jasmine/tests/plot_interact_test.js +++ b/test/jasmine/tests/plot_interact_test.js @@ -568,8 +568,8 @@ describe('plot svg clip paths', function() { d3SelectAll('[clip-path]').each(function() { var cp = d3Select(this).attr('clip-path'); - expect(cp.substring(0, 5 + href.length)).toEqual('url(' + href + '#'); - expect(cp.substring(cp.length - 1)).toEqual(')'); + expect(cp.substring(0, 6 + href.length)).toEqual('url(\'' + href + '#'); + expect(cp.substring(cp.length - 2)).toEqual('\')'); }); base.remove(); diff --git a/test/jasmine/tests/snapshot_test.js b/test/jasmine/tests/snapshot_test.js index 4335cc66a62..1cebfbadb9e 100644 --- a/test/jasmine/tests/snapshot_test.js +++ b/test/jasmine/tests/snapshot_test.js @@ -251,7 +251,7 @@ describe('Plotly.Snapshot', function() { describe('should handle quoted style properties', function() { function checkURL(actual, msg) { - // which is enough to check that toSVG did its job right + // which is enough tot check that toSVG did its job right expect((actual || '').substr(0, 6)).toBe('url(\"#', msg); }