tdf#156034: check for css style further up in the hierarchy

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

4 files changed, 115 insertions, 67 deletions

diff --git a/svgio/inc/svgnode.hxx b/svgio/inc/svgnode.hxx
index f2eb0fcb1b6e..af23bc4eb32f 100644
--- a/svgio/inc/svgnode.hxx
+++ b/svgio/inc/svgnode.hxx
@@ -120,6 +120,9 @@ namespace svgio::svgreader
 
             /// helper for filling the CssStyle vector once dependent on mbCssStyleVectorBuilt
             void fillCssStyleVector(const OUString& rClassStr, const SvgStyleAttributes& rOriginal);
+            void addCssStyle(
+                const SvgDocument& rDocument,
+                const OUString& aConcatenated);
             void fillCssStyleVectorUsingHierarchyAndSelectors(
                 const OUString& rClassStr,
                 const SvgNode& rCurrent,
diff --git a/svgio/qa/cppunit/SvgImportTest.cxx b/svgio/qa/cppunit/SvgImportTest.cxx
index d29cc30bf41c..8e7d86c145f2 100644
--- a/svgio/qa/cppunit/SvgImportTest.cxx
+++ b/svgio/qa/cppunit/SvgImportTest.cxx
@@ -320,6 +320,25 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf145896)
     assertXPath(pDocument, "/primitive2D/transform/polypolygoncolor[3]", "color", "#0000ff");
 }
 
+CPPUNIT_TEST_FIXTURE(Test, testTdf156034)
+{
+    Primitive2DSequence aSequence = parseSvg(u"/svgio/qa/cppunit/data/tdf156034.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");
+}
+
 CPPUNIT_TEST_FIXTURE(Test, testTdf156018)
 {
     Primitive2DSequence aSequence = parseSvg(u"/svgio/qa/cppunit/data/tdf156018.svg");
diff --git a/svgio/qa/cppunit/data/tdf156034.svg b/svgio/qa/cppunit/data/tdf156034.svg
new file mode 100644
index 000000000000..ed14c5fe4aaf
--- /dev/null
+++ b/svgio/qa/cppunit/data/tdf156034.svg
@@ -0,0 +1,20 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" viewBox="-0 0 300 300">
+
+    <style>
+        .g1 rect {fill:green;}
+        #g3 rect {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>
+</svg>
diff --git a/svgio/source/svgreader/svgnode.cxx b/svgio/source/svgreader/svgnode.cxx
index 2137209d5599..35277ef0ed87 100644
--- a/svgio/source/svgreader/svgnode.cxx
+++ b/svgio/source/svgreader/svgnode.cxx
@@ -39,53 +39,30 @@ namespace svgio::svgreader
             return nullptr;
         }
 
-        void SvgNode::fillCssStyleVectorUsingHierarchyAndSelectors(
-            const OUString& rClassStr,
-            const SvgNode& rCurrent,
-            const OUString& aConcatenated)
+        void SvgNode::addCssStyle(const SvgDocument& rDocument, const OUString& aConcatenated)
         {
-            const SvgDocument& rDocument = getDocument();
+            const SvgStyleAttributes* pNew = rDocument.findGlobalCssStyleAttributes(aConcatenated);
 
-            if(!rDocument.hasGlobalCssStyleAttributes())
-                return;
-
-            const SvgNode* pParent = rCurrent.getParent();
-
-            // check for ID (highest priority)
-            if(rCurrent.getId())
+            if(pNew)
             {
-                const OUString& rId = *rCurrent.getId();
-
-                if(rId.getLength())
-                {
-                    const OUString aNewConcatenated(
-                        "#" + rId + aConcatenated);
-
-                    if(pParent)
-                    {
-                        // check for combined selectors at parent firstso that higher specificity will be in front
-                        fillCssStyleVectorUsingHierarchyAndSelectors(rClassStr, *pParent, aNewConcatenated);
-                    }
-
-                    const SvgStyleAttributes* pNew = rDocument.findGlobalCssStyleAttributes(aNewConcatenated);
-
-                    if(pNew)
-                    {
-                        // add CssStyle if found
-                        maCssStyleVector.push_back(pNew);
-                    }
-                }
+                // add CssStyle if found
+                maCssStyleVector.push_back(pNew);
             }
+        }
+
+namespace {
+        std::vector< OUString > parseClass(const SvgNode& rNode)
+        {
+            std::vector< OUString > aParts;
 
             // check for 'class' references (a list of entries is allowed)
-            if(rCurrent.getClass())
+            if(rNode.getClass())
             {
-                const OUString& rClassList = *rCurrent.getClass();
+                const OUString& rClassList = *rNode.getClass();
                 const sal_Int32 nLen(rClassList.getLength());
 
                 if(nLen)
                 {
-                    std::vector< OUString > aParts;
                     sal_Int32 nPos(0);
                     OUStringBuffer aToken;
 
@@ -108,25 +85,68 @@ namespace svgio::svgreader
                             nPos++;
                         }
                     }
+                }
+            }
 
-                    for(const auto &a : aParts)
+            return aParts;
+        }
+} //namespace
+
+        void SvgNode::fillCssStyleVectorUsingHierarchyAndSelectors(
+            const OUString& rClassStr,
+            const SvgNode& rCurrent,
+            const OUString& aConcatenated)
+        {
+            const SvgDocument& rDocument = getDocument();
+
+            if(!rDocument.hasGlobalCssStyleAttributes())
+                return;
+
+            const SvgNode* pParent = rCurrent.getParent();
+
+            // check for ID (highest priority)
+            if(rCurrent.getId())
+            {
+                const OUString& rId = *rCurrent.getId();
+
+                if(rId.getLength())
+                {
+                    const OUString aNewConcatenated(
+                        "#" + rId + aConcatenated);
+                    if(pParent)
                     {
-                        const OUString aNewConcatenated(
-                            "." + a + aConcatenated);
+                        // check for combined selectors at parent first so that higher specificity will be in front
+                        fillCssStyleVectorUsingHierarchyAndSelectors(rClassStr, *pParent, aNewConcatenated);
+                    }
+                    addCssStyle(rDocument, aNewConcatenated);
 
-                        if(pParent)
-                        {
-                            // check for combined selectors at parent firstso that higher specificity will be in front
-                            fillCssStyleVectorUsingHierarchyAndSelectors(rClassStr, *pParent, aNewConcatenated);
-                        }
+                    // look further up in the hierarchy
+                    if(pParent && pParent->getId())
+                    {
+                        const OUString& rParentId = pParent->getId().value();
+                        addCssStyle(rDocument, "#" + rParentId + aConcatenated);
+                    }
+                }
+            }
 
-                        const SvgStyleAttributes* pNew = rDocument.findGlobalCssStyleAttributes(aNewConcatenated);
+            std::vector <OUString> aClasses = parseClass(rCurrent);
+            for(const auto &aClass : aClasses)
+            {
+                const OUString aNewConcatenated("." + aClass + aConcatenated);
+                if(pParent)
+                {
+                    // check for combined selectors at parent first so that higher specificity will be in front
+                    fillCssStyleVectorUsingHierarchyAndSelectors(rClassStr, *pParent, aNewConcatenated);
+                }
+                addCssStyle(rDocument, aNewConcatenated);
 
-                        if(pNew)
-                        {
-                            // add CssStyle if found
-                            maCssStyleVector.push_back(pNew);
-                        }
+                // look further up in the hierarchy
+                if(pParent)
+                {
+                    std::vector <OUString> aParentClasses = parseClass(*pParent);
+                    for(const auto &aParentClass : aParentClasses)
+                    {
+                        addCssStyle(rDocument, "." + aParentClass + aConcatenated);
                     }
                 }
             }
@@ -149,31 +169,17 @@ namespace svgio::svgreader
 
             if(pParent)
             {
-                // check for combined selectors at parent firstso that higher specificity will be in front
+                // check for combined selectors at parent first so that higher specificity will be in front
                 fillCssStyleVectorUsingHierarchyAndSelectors(rClassStr, *pParent, aNewConcatenated);
             }
 
-            const SvgStyleAttributes* pNew = rDocument.findGlobalCssStyleAttributes(aNewConcatenated);
-
-            if(pNew)
-            {
-                // add CssStyle if found
-                maCssStyleVector.push_back(pNew);
-            }
+            addCssStyle(rDocument, aNewConcatenated);
 
             // check if there is a css style with element inside element
             if(pParent)
             {
                 OUString sParentType(SVGTokenToStr(pParent->getType()));
-
-                aNewConcatenated = sParentType + rClassStr;
-                pNew = rDocument.findGlobalCssStyleAttributes(aNewConcatenated);
-
-                if(pNew)
-                {
-                    // add CssStyle if found
-                    maCssStyleVector.push_back(pNew);
-                }
+                addCssStyle(rDocument, sParentType + rClassStr);
             }
         }