From 24591c1c22011ad8b444110e49ff1f37aec79b9c Mon Sep 17 00:00:00 2001 From: John Kenny Date: Wed, 3 Jan 2024 08:41:22 -0800 Subject: [PATCH 1/4] Fixed bug in removeHiddenElements. --- plugins/removeHiddenElems.js | 31 +++++++++++++++++---------- test/plugins/removeHiddenElems.16.svg | 23 ++++++++++++++++++++ test/plugins/removeHiddenElems.17.svg | 23 ++++++++++++++++++++ 3 files changed, 66 insertions(+), 11 deletions(-) create mode 100644 test/plugins/removeHiddenElems.16.svg create mode 100644 test/plugins/removeHiddenElems.17.svg diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index 75720a068..320977366 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -93,6 +93,23 @@ exports.fn = (root, params) => { */ let deoptimized = false; + /** + * Nodes can't be removed if they or any of their children have an id attribute that is referenced. + * @param {XastElement} node + * @returns boolean + */ + function canRemoveNonRenderingNode(node) { + if (allReferences.has(node.attributes.id)) { + return false; + } + for (const child of node.children) { + if (child.type === 'element' && !canRemoveNonRenderingNode(child)) { + return false; + } + } + return true; + } + /** * @param {XastChild} node * @param {XastParent} parentNode @@ -114,13 +131,7 @@ exports.fn = (root, params) => { element: { enter: (node, parentNode) => { // transparent non-rendering elements still apply where referenced - if (nonRendering.has(node.name)) { - if (node.attributes.id == null) { - detachNodeFromParent(node, parentNode); - return visitSkip; - } - - nonRenderedNodes.set(node, parentNode); + if (nonRendering.has(node.name) || node.name === 'defs') { return visitSkip; } const computedStyle = computeStyle(stylesheet, node); @@ -421,16 +432,14 @@ exports.fn = (root, params) => { nonRenderedNode, nonRenderedParent, ] of nonRenderedNodes.entries()) { - const id = nonRenderedNode.attributes.id; - - if (!allReferences.has(id)) { + if (canRemoveNonRenderingNode(nonRenderedNode)) { detachNodeFromParent(nonRenderedNode, nonRenderedParent); } } } for (const [node, parentNode] of allDefs.entries()) { - if (node.children.length === 0) { + if (canRemoveNonRenderingNode(node)) { detachNodeFromParent(node, parentNode); } } diff --git a/test/plugins/removeHiddenElems.16.svg b/test/plugins/removeHiddenElems.16.svg new file mode 100644 index 000000000..fb29f9b8b --- /dev/null +++ b/test/plugins/removeHiddenElems.16.svg @@ -0,0 +1,23 @@ +Don't remove used symbols. + +=== + + + + + + + + + + +@@@ + + + + + + + + + \ No newline at end of file diff --git a/test/plugins/removeHiddenElems.17.svg b/test/plugins/removeHiddenElems.17.svg new file mode 100644 index 000000000..ef3557e0a --- /dev/null +++ b/test/plugins/removeHiddenElems.17.svg @@ -0,0 +1,23 @@ +Don't remove used symbols. + +=== + + + + + + + + + + +@@@ + + + + + + + + + \ No newline at end of file From 02ae77d92a4ef2b0f37925625b6ed422331de783 Mon Sep 17 00:00:00 2001 From: John Kenny Date: Wed, 3 Jan 2024 09:29:52 -0800 Subject: [PATCH 2/4] Updated test case description. --- test/plugins/removeHiddenElems.16.svg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/plugins/removeHiddenElems.16.svg b/test/plugins/removeHiddenElems.16.svg index fb29f9b8b..9795db0b9 100644 --- a/test/plugins/removeHiddenElems.16.svg +++ b/test/plugins/removeHiddenElems.16.svg @@ -1,4 +1,4 @@ -Don't remove used symbols. +Don't remove non-rendering elements if children have IDs. === From ac57ffdab01a1794954a34c562e1bc75528cc222 Mon Sep 17 00:00:00 2001 From: John Kenny Date: Wed, 3 Jan 2024 12:55:48 -0800 Subject: [PATCH 3/4] Added test case and reverted some changes to removeHiddenElems. --- plugins/removeHiddenElems.js | 5 +++-- test/plugins/removeHiddenElems.18.svg | 23 +++++++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) create mode 100644 test/plugins/removeHiddenElems.18.svg diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index 320977366..aec69c7b7 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -131,7 +131,8 @@ exports.fn = (root, params) => { element: { enter: (node, parentNode) => { // transparent non-rendering elements still apply where referenced - if (nonRendering.has(node.name) || node.name === 'defs') { + if (nonRendering.has(node.name)) { + nonRenderedNodes.set(node, parentNode); return visitSkip; } const computedStyle = computeStyle(stylesheet, node); @@ -439,7 +440,7 @@ exports.fn = (root, params) => { } for (const [node, parentNode] of allDefs.entries()) { - if (canRemoveNonRenderingNode(node)) { + if (node.children.length === 0) { detachNodeFromParent(node, parentNode); } } diff --git a/test/plugins/removeHiddenElems.18.svg b/test/plugins/removeHiddenElems.18.svg new file mode 100644 index 000000000..e24700598 --- /dev/null +++ b/test/plugins/removeHiddenElems.18.svg @@ -0,0 +1,23 @@ +XXXXXXXXX + +=== + + + + + + + + + + +@@@ + + + + + + + + + From b02d26c3038f60782ab84cc3bd52988cfa64fe31 Mon Sep 17 00:00:00 2001 From: John Kenny Date: Wed, 3 Jan 2024 16:50:12 -0800 Subject: [PATCH 4/4] Added test case description. --- test/plugins/removeHiddenElems.18.svg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/plugins/removeHiddenElems.18.svg b/test/plugins/removeHiddenElems.18.svg index e24700598..ad1bb7f34 100644 --- a/test/plugins/removeHiddenElems.18.svg +++ b/test/plugins/removeHiddenElems.18.svg @@ -1,4 +1,4 @@ -XXXXXXXXX +Preserve with referenced path. ===