Merge pull request #3725 from blubiszirp/fix3724-parenthesis-in-url-draws-data-outside-plot-area

Fix3724 parenthesis in url draws data outside plot area

Author: etpinard

src/components/drawing/index.js

@@ -1028,7 +1028,7 @@ drawing.setClipUrl = function(s, localId, gd) {
     var context = gd._context;
     var baseUrl = context._exportedPlot ? '' : (context._baseUrl || '');
 
-    s.attr('clip-path', 'url(' + baseUrl + '#' + localId + ')');
+    s.attr('clip-path', 'url(\'' + baseUrl + '#' + localId + '\')');
 };
 
 drawing.getTranslate = function(element) {

test/jasmine/assets/custom_assertions.js

@@ -226,8 +226,8 @@ exports.assertClip = function(sel, isClipped, size, msg) {
         var clipPath = d3.select(this).attr('clip-path');
 
         if(isClipped) {
-            expect(String(clipPath).substr(0, 4))
-                .toBe('url(', msg + ' clip path ' + '(item ' + i + ')');
+            expect(String(clipPath).substr(0, 5))
+                .toBe('url(\'', msg + ' clip path ' + '(item ' + i + ')');
         } else {
             expect(clipPath)
                 .toBe(null, msg + ' clip path ' + '(item ' + i + ')');

test/jasmine/assets/get_bbox.js

@@ -15,7 +15,7 @@ module.exports = function getBBox(element) {
     if(!clipPathAttr) return elementBBox;
 
     // only supports 'url(#<id>)' at the moment
-    var clipPathId = clipPathAttr.substring(5, clipPathAttr.length - 1);
+    var clipPathId = clipPathAttr.substring(6, clipPathAttr.length - 2);
     var clipBBox = getClipBBox(clipPathId);
 
     return minBBox(elementBBox, clipBBox);
@@ -28,8 +28,7 @@ function getClipBBox(clipPathId) {
     try {
         // this line throws an error in FF (38 and 45 at least)
         clipBBox = clipPath.node().getBBox();
-    }
-    catch(e) {
+    } catch(e) {
         // use DOM attributes as fallback
         var path = d3.select(clipPath.node().firstChild);
 

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);
 

test/jasmine/tests/layout_images_test.js

@@ -448,22 +448,22 @@ 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(d3.select('image').attr('clip-path') || '').toMatch(/xy\)$/);
+        expect(d3.select('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(d3.select('image').attr('clip-path') || '').toMatch(/xy2\)$/);
+            expect(d3.select('image').attr('clip-path') || '').toMatch(/xy2\'\)$/);
 
             // log to paper
             return Plotly.relayout(gd, {'images[0].yref': 'paper'});
         })
         .then(function() {
             expect(gd.layout.images[0].y).toBe(1);
 
-            expect(d3.select('image').attr('clip-path') || '').toMatch(/x\)$/);
+            expect(d3.select('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'});
@@ -477,7 +477,7 @@ describe('images log/linear axis changes', function() {
         .then(function() {
             expect(gd.layout.images[0].y).toBe(1);
 
-            expect(d3.select('image').attr('clip-path') || '').toMatch(/^[^x]+y2\)$/);
+            expect(d3.select('image').attr('clip-path') || '').toMatch(/^[^x]+y2\'\)$/);
 
             // log to linear
             return Plotly.relayout(gd, {'images[0].yref': 'y'});

test/jasmine/tests/plot_interact_test.js

@@ -545,8 +545,8 @@ describe('plot svg clip paths', function() {
             d3.selectAll('[clip-path]').each(function() {
                 var cp = d3.select(this).attr('clip-path');
 
-                expect(cp.substring(0, 5)).toEqual('url(#');
-                expect(cp.substring(cp.length - 1)).toEqual(')');
+                expect(cp.substring(0, 6)).toEqual('url(\'#');
+                expect(cp.substring(cp.length - 2)).toEqual('\')');
             });
         })
         .catch(failTest)
@@ -569,8 +569,8 @@ describe('plot svg clip paths', function() {
             d3.selectAll('[clip-path]').each(function() {
                 var cp = d3.select(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();

test/jasmine/tests/shapes_test.js

@@ -458,7 +458,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',
@@ -475,7 +475,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',
@@ -484,7 +484,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\'\)$/);
         })
         .catch(failTest)
         .then(done);

test/jasmine/tests/toimage_test.js

@@ -229,10 +229,10 @@ describe('Plotly.toImage', function() {
             var clipPath = gSubplot.getAttribute('clip-path');
             var len = clipPath.length;
 
-            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 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 middle = clipPath.substr(4, 10);
             expect(middle.length).toBe(10, 'subplot clipPath uid length');