From 3aeb0ceee8a51942abbb3854bded917f77610258 Mon Sep 17 00:00:00 2001
From: Andrea Bogazzi <andreabogazzi79@gmail.com>
Date: Tue, 21 Apr 2020 10:34:07 +0200
Subject: [PATCH] fix(svg_import): Fix some parsing logic for nested SVGs.
 (#6284)

---
 src/parser.js                      |  27 ++++++++++++++++------
 test/visual/assets/nested-svgs.svg |  36 +++++++++++++++++++++++++++++
 test/visual/golden/nested-svgs.png | Bin 0 -> 678 bytes
 test/visual/svg_import.js          |   2 ++
 4 files changed, 58 insertions(+), 7 deletions(-)
 create mode 100644 test/visual/assets/nested-svgs.svg
 create mode 100644 test/visual/golden/nested-svgs.png

diff --git a/src/parser.js b/src/parser.js
index 504685e1f26..e4ee4bfca34 100644
--- a/src/parser.js
+++ b/src/parser.js
@@ -534,7 +534,9 @@
    * Add a <g> element that envelop all child elements and makes the viewbox transformMatrix descend on all elements
    */
   function applyViewboxTransform(element) {
-
+    if (!fabric.svgViewBoxElementsRegEx.test(element.nodeName)) {
+      return;
+    }
     var viewBoxAttr = element.getAttribute('viewBox'),
         scaleX = 1,
         scaleY = 1,
@@ -546,8 +548,7 @@
         x = element.getAttribute('x') || 0,
         y = element.getAttribute('y') || 0,
         preserveAspectRatio = element.getAttribute('preserveAspectRatio') || '',
-        missingViewBox = (!viewBoxAttr || !fabric.svgViewBoxElementsRegEx.test(element.nodeName)
-                           || !(viewBoxAttr = viewBoxAttr.match(reViewBoxAttrValue))),
+        missingViewBox = (!viewBoxAttr || !(viewBoxAttr = viewBoxAttr.match(reViewBoxAttrValue))),
         missingDimAttr = (!widthAttr || !heightAttr || widthAttr === '100%' || heightAttr === '100%'),
         toBeParsed = missingViewBox && missingDimAttr,
         parsedDim = { }, translateMatrix = '', widthDiff = 0, heightDiff = 0;
@@ -556,6 +557,16 @@
     parsedDim.height = 0;
     parsedDim.toBeParsed = toBeParsed;
 
+    if (missingViewBox) {
+      if (((x || y) && element.parentNode.nodeName !== '#document')) {
+        translateMatrix = ' translate(' + parseUnit(x) + ' ' + parseUnit(y) + ') ';
+        matrix = (element.getAttribute('transform') || '') + translateMatrix;
+        element.setAttribute('transform', matrix);
+        element.removeAttribute('x');
+        element.removeAttribute('y');
+      }
+    }
+
     if (toBeParsed) {
       return parsedDim;
     }
@@ -563,6 +574,7 @@
     if (missingViewBox) {
       parsedDim.width = parseUnit(widthAttr);
       parsedDim.height = parseUnit(heightAttr);
+      // set a transform for elements that have x y and are inner(only) SVGs
       return parsedDim;
     }
     minX = -parseFloat(viewBoxAttr[1]);
@@ -615,8 +627,7 @@
     if (scaleX === 1 && scaleY === 1 && minX === 0 && minY === 0 && x === 0 && y === 0) {
       return parsedDim;
     }
-
-    if (x || y) {
+    if ((x || y) && element.parentNode.nodeName !== '#document') {
       translateMatrix = ' translate(' + parseUnit(x) + ' ' + parseUnit(y) + ') ';
     }
 
@@ -626,7 +637,8 @@
                   scaleY + ' ' +
                   (minX * scaleX + widthDiff) + ' ' +
                   (minY * scaleY + heightDiff) + ') ';
-    parsedDim.viewboxTransform = fabric.parseTransformAttribute(matrix);
+    // seems unused.
+    // parsedDim.viewboxTransform = fabric.parseTransformAttribute(matrix);
     if (element.nodeName === 'svg') {
       el = element.ownerDocument.createElementNS(fabric.svgNS, 'g');
       // element.firstChild != null
@@ -637,6 +649,8 @@
     }
     else {
       el = element;
+      el.removeAttribute('x');
+      el.removeAttribute('y');
       matrix = el.getAttribute('transform') + matrix;
     }
     el.setAttribute('transform', matrix);
@@ -694,7 +708,6 @@
       return fabric.svgValidTagNamesRegEx.test(el.nodeName.replace('svg:', '')) &&
             !hasAncestorWithNodeName(el, fabric.svgInvalidAncestorsRegEx); // http://www.w3.org/TR/SVG/struct.html#DefsElement
     });
-
     if (!elements || (elements && !elements.length)) {
       callback && callback([], {});
       return;
diff --git a/test/visual/assets/nested-svgs.svg b/test/visual/assets/nested-svgs.svg
new file mode 100644
index 00000000000..2c70193ed24
--- /dev/null
+++ b/test/visual/assets/nested-svgs.svg
@@ -0,0 +1,36 @@
+<!-- Expected: X & Y will be ignored in case of the outermost svg element -->
+<svg xmlns="http://www.w3.org/2000/svg" x="10" y="10" width="100" height="100" viewBox="0 0 100 100">
+		<!-- Expected: A green and magenta rect with a size of 12.5x25 will be visible at left/top -->
+    <svg viewBox="100 100 25 25" width="25" height="25">
+        <rect x="100" y="100" width="12.5" height="25" fill="#00FF00"/>
+        <!-- the viewbox causes that only a part (width="12.5") of the following rect is visible-->
+        <rect x="112.5" y="100" width="25" height="25" fill="#FF00FF"/>
+    </svg>
+
+		<!-- Expected:
+         - A blue and black rect (diagonally shifted) with a size of 25x25 will be visible at right/bottom corner.
+         - A turquoise rect with a size of 12.5x12.5 will be visible below of the blue rect
+     -->
+     <svg y="50" >
+         <rect x="0" y="0" width="25" height="25" fill="#00FFFF"/>
+         <rect x="25" y="25" width="25" height="25" fill="#FF0000"/>
+
+         <svg x="0" y="25" width="12.5" height="12.5">
+             <rect x="0" y="0" width="25" height="25" fill="#FFFF00"/>
+         </svg>
+     </svg>
+    <svg x="50" y="50" width="50" height="50">
+        <rect x="0" y="0" width="25" height="25" fill="#0000FF"/>
+        <rect x="25" y="25" width="25" height="25" fill="#000000"/>
+
+        <svg x="0" y="25" width="12.5" height="12.5">
+            <rect x="0" y="0" width="25" height="25" fill="#00FFFF"/>
+        </svg>
+    </svg>
+
+		<!-- Expected: A yellow rect will be drawn to the right of the blue rect. -->
+    <rect x="75" y="50" width="25" height="25" fill="yellow"/>
+
+    <!-- canvas size -->
+    <rect x="0" y="0" width="100" height="100" stroke="#FF0000" stroke-width="2" fill="none"/>
+</svg>
diff --git a/test/visual/golden/nested-svgs.png b/test/visual/golden/nested-svgs.png
new file mode 100644
index 0000000000000000000000000000000000000000..701f8205a1664fd3e5f07fb7783e3aaa2fe57e88
GIT binary patch
literal 678
zcmeAS@N?(olHy`uVBq!ia0vp^DIm<j1|$m}O$`B3Y)RhkE)4%caKYZ?lNlJ8qCH(4
zLn`LHy}Qx-N}){q$M07gXUTD9daFJX@%1~`#G9w<n9TZDly_=(y{k*_qe;=uVbK>#
zoSPjFH*Tq5J0@xt^H0iT&Dl18y9=MK`exUC-?zK{@ihII)t<9D7JJUxzjN*VnEJa*
zGP~aQ`~5Dq<n_HY$MUWCLVHP;V+sarjfNaOjtM+W$pRA12PE(d-E!~G_p3U%!1zbR
z=b(QVezQw^>~C5#=S$PvZtKJM*R8&!U00{)uUAwNz;}W~qj{K$EB39*6yrAkHtEZy
zthZNvO(uJ$P4)@X)zQuU7ZJO6ers!a`f9UFR(2o0ZOBo1sa9F`B_h@A<Xe@`q0EaH
z-?#Vm|33Li&{el0nUsx1Dkt4Oe)fN6a4hMn!*1I*Wme5g45}XGbMz7GRd|3bo2z2J
zDX=VU)#snt^)FAJ-f11m?D;OP+G@L5@#_8dMq%IV*uwdL$!}Ai^k!PFkLu^QDwT7p
zL!~9_vis-P|4y|$S#tH**O_Pkbx(cz@$b%!t2+1GpLe!->*<#{Ur(9Md;UV&XzJZv
zhvT15l2OsLNiZ`sdcpVn*2$2&$Nbin#OKC(nAvE2k0&9Lpf2C?C)sn_{8N6rI%1C5
R9s(vj22WQ%mvv4FO#qOH7`^}i

literal 0
HcmV?d00001

diff --git a/test/visual/svg_import.js b/test/visual/svg_import.js
index 6eff22438cc..f4022ebbcf0 100644
--- a/test/visual/svg_import.js
+++ b/test/visual/svg_import.js
@@ -84,6 +84,8 @@
     'gold-logo',
     'svg_missing_clippath',
     'image-rendering-attr',
+    // this svg below here is not correct. but we do not want additional regressions
+    'nested-svgs'
   ].map(createTestFromSVG);
 
   tests.forEach(visualTestLoop(QUnit));