Skip to content

Commit

Permalink
fix(svg_import): Fix some parsing logic for nested SVGs. (#6284)
Browse files Browse the repository at this point in the history
  • Loading branch information
asturur authored Apr 21, 2020
1 parent 1c7c058 commit 3aeb0ce
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 7 deletions.
27 changes: 20 additions & 7 deletions src/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
Expand All @@ -556,13 +557,24 @@
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;
}

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]);
Expand Down Expand Up @@ -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) + ') ';
}

Expand All @@ -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
Expand All @@ -637,6 +649,8 @@
}
else {
el = element;
el.removeAttribute('x');
el.removeAttribute('y');
matrix = el.getAttribute('transform') + matrix;
}
el.setAttribute('transform', matrix);
Expand Down Expand Up @@ -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;
Expand Down
36 changes: 36 additions & 0 deletions test/visual/assets/nested-svgs.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added test/visual/golden/nested-svgs.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 2 additions & 0 deletions test/visual/svg_import.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down

0 comments on commit 3aeb0ce

Please sign in to comment.