tdf#129356: handle css combinator when the element name is combined...

... with the ID or the class

While at it, simplify the code a bit

Change-Id: I9e36f334b884d31229568835a346d4427a47c760
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/155945
Tested-by: Jenkins
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>

4 files changed, 80 insertions, 47 deletions

diff --git a/svgio/inc/svgnode.hxx b/svgio/inc/svgnode.hxx
index 0d8008d41c90..96c6ade01efa 100644
--- a/svgio/inc/svgnode.hxx
+++ b/svgio/inc/svgnode.hxx
@@ -125,7 +125,7 @@ namespace svgio::svgreader
                 const OUString& aConcatenated);
             void fillCssStyleVectorUsingHierarchyAndSelectors(
                 const SvgNode& rCurrent,
-                const OUString& aConcatenated);
+                std::u16string_view aConcatenated);
             void fillCssStyleVectorUsingParent(
                 const SvgNode& rCurrent);
 
diff --git a/svgio/qa/cppunit/SvgImportTest.cxx b/svgio/qa/cppunit/SvgImportTest.cxx
index 4d7cd9772d51..0450d3617e11 100644
--- a/svgio/qa/cppunit/SvgImportTest.cxx
+++ b/svgio/qa/cppunit/SvgImportTest.cxx
@@ -441,6 +441,29 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf156168)
     assertXPath(pDocument, "/primitive2D/transform/polypolygonstroke[4]/line", "color", "#00ff00");
 }
 
+CPPUNIT_TEST_FIXTURE(Test, testTdf129356)
+{
+    Primitive2DSequence aSequence = parseSvg(u"/svgio/qa/cppunit/data/tdf129356.svg");
+    CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(aSequence.getLength()));
+
+    drawinglayer::Primitive2dXmlDump dumper;
+    xmlDocUniquePtr pDocument = dumper.dumpAndParse(aSequence);
+
+    CPPUNIT_ASSERT (pDocument);
+
+    // Without the fix in place, this test would have failed with
+    // - Expected: #008000
+    // - Actual  : #0000ff
+    assertXPath(pDocument, "/primitive2D/transform/polypolygoncolor[1]", "color", "#008000");
+    assertXPath(pDocument, "/primitive2D/transform/polypolygoncolor[2]", "color", "#008000");
+    assertXPath(pDocument, "/primitive2D/transform/polypolygoncolor[3]", "color", "#008000");
+    assertXPath(pDocument, "/primitive2D/transform/polypolygoncolor[4]", "color", "#008000");
+    assertXPath(pDocument, "/primitive2D/transform/polypolygoncolor[5]", "color", "#008000");
+    assertXPath(pDocument, "/primitive2D/transform/polypolygoncolor[6]", "color", "#008000");
+    assertXPath(pDocument, "/primitive2D/transform/polypolygoncolor[7]", "color", "#008000");
+    assertXPath(pDocument, "/primitive2D/transform/polypolygoncolor[8]", "color", "#008000");
+}
+
 CPPUNIT_TEST_FIXTURE(Test, testTdf156034)
 {
     Primitive2DSequence aSequence = parseSvg(u"/svgio/qa/cppunit/data/tdf156034.svg");
diff --git a/svgio/qa/cppunit/data/tdf129356.svg b/svgio/qa/cppunit/data/tdf129356.svg
new file mode 100644
index 000000000000..46bd6935daa4
--- /dev/null
+++ b/svgio/qa/cppunit/data/tdf129356.svg
@@ -0,0 +1,34 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" viewBox="-0 0 800 800">
+
+    <style>
+        g.g1 rect {fill:green;}
+        g#g3 rect {fill:green;}
+        g.g4 #r1 {fill:green;}
+        g#g3 .r5 {fill:green;}
+    </style>
+
+    <g class="g4 g1">
+        <g class="g2">
+            <rect x="0" y="0" height="50" width="50" fill="blue"></rect>
+        </g>
+        <rect x="60" y="0" height="50" width="50" fill="blue"></rect>
+    </g>
+    <g id="g3">
+        <g id="g4">
+            <rect x="120" y="0" height="50" width="50" fill="blue"></rect>
+        </g>
+        <rect x="180" y="0" height="50" width="50" fill="blue"></rect>
+    </g>
+    <g class="g4 g1">
+        <g>
+            <rect id="r1" x="240" y="0" height="50" width="50" fill="blue"></rect>
+        </g>
+        <rect id="r1" x="300" y="0" height="50" width="50" fill="blue"></rect>
+    </g>
+    <g id="g3">
+        <g id="g4">
+            <rect class="r5" x="360" y="0" height="50" width="50" fill="blue"></rect>
+        </g>
+        <rect class="r5" x="420" y="0" height="50" width="50" fill="blue"></rect>
+    </g>
+</svg>
diff --git a/svgio/source/svgreader/svgnode.cxx b/svgio/source/svgreader/svgnode.cxx
index 4f82a16ebf1e..fd80337eb5b6 100644
--- a/svgio/source/svgreader/svgnode.cxx
+++ b/svgio/source/svgreader/svgnode.cxx
@@ -94,7 +94,7 @@ namespace {
 
         void SvgNode::fillCssStyleVectorUsingHierarchyAndSelectors(
             const SvgNode& rCurrent,
-            const OUString& aConcatenated)
+            std::u16string_view aConcatenated)
         {
             const SvgDocument& rDocument = getDocument();
 
@@ -102,6 +102,7 @@ namespace {
                 return;
 
             const SvgNode* pParent = rCurrent.getParent();
+            OUString sCurrentType(SVGTokenToStr(rCurrent.getType()));
 
             // check for ID (highest priority)
             if(rCurrent.getId())
@@ -110,21 +111,17 @@ namespace {
 
                 if(rId.getLength())
                 {
-                    const OUString aNewConcatenated(
-                        "#" + rId + aConcatenated);
+                    const OUString aNewConcatenated("#" + rId + aConcatenated);
+                    addCssStyle(rDocument, aNewConcatenated);
+
+                    if(!sCurrentType.isEmpty())
+                        addCssStyle(rDocument, sCurrentType + aNewConcatenated);
+
                     if(pParent)
                     {
                         // check for combined selectors at parent first so that higher specificity will be in front
                         fillCssStyleVectorUsingHierarchyAndSelectors(*pParent, aNewConcatenated);
                     }
-                    addCssStyle(rDocument, aNewConcatenated);
-
-                    // look further up in the hierarchy
-                    if(!aConcatenated.isEmpty() && pParent && pParent->getId())
-                    {
-                        const OUString& rParentId = pParent->getId().value();
-                        addCssStyle(rDocument, "#" + rParentId + aConcatenated);
-                    }
                 }
             }
 
@@ -132,55 +129,34 @@ namespace {
             for(const auto &aClass : aClasses)
             {
                 const OUString aNewConcatenated("." + aClass + aConcatenated);
+                addCssStyle(rDocument, aNewConcatenated);
+
+                if(!sCurrentType.isEmpty())
+                    addCssStyle(rDocument, sCurrentType + aNewConcatenated);
+
                 if(pParent)
                 {
                     // check for combined selectors at parent first so that higher specificity will be in front
                     fillCssStyleVectorUsingHierarchyAndSelectors(*pParent, aNewConcatenated);
                 }
-                addCssStyle(rDocument, aNewConcatenated);
+            }
 
-                // look further up in the hierarchy
-                if(!aConcatenated.isEmpty() && pParent)
-                {
-                    std::vector <OUString> aParentClasses = parseClass(*pParent);
-                    for(const auto &aParentClass : aParentClasses)
-                    {
-                        addCssStyle(rDocument, "." + aParentClass + aConcatenated);
-                    }
-                }
+            if(!sCurrentType.isEmpty())
+            {
+                const OUString aNewConcatenated(sCurrentType + aConcatenated);
+                addCssStyle(rDocument, aNewConcatenated);
             }
 
-            OUString sCurrentType(SVGTokenToStr(getType()));
+            OUString sType(SVGTokenToStr(getType()));
 
             // check for class-dependent references to CssStyles
-            if(sCurrentType.isEmpty())
+            if(sType.isEmpty())
                 return;
 
-            OUString aNewConcatenated(aConcatenated);
-
-            if(!rCurrent.getId() && !rCurrent.getClass() && 0 == aConcatenated.indexOf(sCurrentType))
-            {
-                // no new CssStyle Selector and already starts with sCurrentType, do not concatenate;
-                // we pass an 'empty' node (in the sense of CssStyle Selector)
-            }
-            else
-            {
-                aNewConcatenated = sCurrentType + aConcatenated;
-            }
-
             if(pParent)
             {
                 // check for combined selectors at parent first so that higher specificity will be in front
-                fillCssStyleVectorUsingHierarchyAndSelectors(*pParent, aNewConcatenated);
-            }
-
-            addCssStyle(rDocument, aNewConcatenated);
-
-            // check if there is a css style with element inside element
-            if(pParent)
-            {
-                OUString sParentType(SVGTokenToStr(pParent->getType()));
-                addCssStyle(rDocument, sParentType + sCurrentType);
+                fillCssStyleVectorUsingHierarchyAndSelectors(*pParent, sType);
             }
         }
 
@@ -309,7 +285,7 @@ namespace {
             fillCssStyleVectorUsingParent(*this);
 
             // check the hierarchy for concatenated patterns of Selectors
-            fillCssStyleVectorUsingHierarchyAndSelectors(*this, OUString());
+            fillCssStyleVectorUsingHierarchyAndSelectors(*this, std::u16string_view());
 
 
             // tdf#99115, Add css selector '*' style only if the element is on top of the hierarchy