tdf#156251: Add gap between text elements when needed

Partially revert a42f5faac7c6d4590e632cf40e3ba9eb618e6f56
"tdf#103888: Do not add a gap at the end of each text portion"
and adapt code to keep tdf#103888 fixed

Change-Id: I4b3f1ff7d87b1945233d9b05824d58af1e001d65
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/154364
Tested-by: Jenkins
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
(cherry picked from commit 5079e7937ef471a44dcf119dc6ae0a334d9c6adc)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/154326
Reviewed-by: Stéphane Guillou <stephane.guillou@libreoffice.org>

7 files changed, 107 insertions, 23 deletions

diff --git a/svgio/inc/svgcharacternode.hxx b/svgio/inc/svgcharacternode.hxx
index 738ddf4d9e73..5fad2008ba1c 100644
--- a/svgio/inc/svgcharacternode.hxx
+++ b/svgio/inc/svgcharacternode.hxx
@@ -122,6 +122,9 @@ namespace svgio::svgreader
             /// the string data
             OUString           maText;
 
+            // keep a copy of string data before space handling
+            OUString           maTextBeforeSpaceHandling;
+
             /// local helpers
             rtl::Reference<drawinglayer::primitive2d::BasePrimitive2D> createSimpleTextPrimitive(
                 SvgTextPosition& rSvgTextPosition,
@@ -141,10 +144,13 @@ namespace svgio::svgreader
             virtual const SvgStyleAttributes* getSvgStyleAttributes() const override;
             void decomposeText(drawinglayer::primitive2d::Primitive2DContainer& rTarget, SvgTextPosition& rSvgTextPosition) const;
             void whiteSpaceHandling();
+            void addGap();
             void concatenate(std::u16string_view rText);
 
             /// Text content
             const OUString& getText() const { return maText; }
+
+            const OUString& getTextBeforeSpaceHandling() const { return maTextBeforeSpaceHandling; }
         };
 
 } // end of namespace svgio::svgreader
diff --git a/svgio/inc/svgtools.hxx b/svgio/inc/svgtools.hxx
index c395702e5998..91ea53ce5e28 100644
--- a/svgio/inc/svgtools.hxx
+++ b/svgio/inc/svgtools.hxx
@@ -127,7 +127,6 @@ namespace svgio::svgreader
         void readImageLink(const OUString& rCandidate, OUString& rXLink, OUString& rUrl, OUString& rData);
 
         OUString consolidateContiguousSpace(const OUString& rCandidate);
-        OUString xmlSpaceHandling(const OUString& rCandidate, bool bIsDefault);
 
         // #125325# removes block comment of the general form '/* ... */', returns
         // an adapted string or the original if no comments included
diff --git a/svgio/qa/cppunit/SvgImportTest.cxx b/svgio/qa/cppunit/SvgImportTest.cxx
index ee21d95c1970..46164357691b 100644
--- a/svgio/qa/cppunit/SvgImportTest.cxx
+++ b/svgio/qa/cppunit/SvgImportTest.cxx
@@ -733,7 +733,7 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf85770)
     assertXPath(pDocument, "/primitive2D/transform/textsimpleportion[1]", "height", "11");
     assertXPath(pDocument, "/primitive2D/transform/textsimpleportion[1]", "familyname", "Times New Roman");
     assertXPath(pDocument, "/primitive2D/transform/textsimpleportion[2]", "fontcolor", "#000000");
-    assertXPath(pDocument, "/primitive2D/transform/textsimpleportion[2]", "text", "Start");
+    assertXPath(pDocument, "/primitive2D/transform/textsimpleportion[2]", "text", "Start ");
     assertXPath(pDocument, "/primitive2D/transform/textsimpleportion[2]", "height", "11");
     assertXPath(pDocument, "/primitive2D/transform/textsimpleportion[2]", "familyname", "Times New Roman");
     assertXPath(pDocument, "/primitive2D/transform/textsimpleportion[3]", "fontcolor", "#000000");
@@ -1146,6 +1146,27 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf103888)
     assertXPath(pDocument, "/primitive2D/transform/transform/textsimpleportion[3]", "text", "hebung");
 }
 
+CPPUNIT_TEST_FIXTURE(Test, testTdf156251)
+{
+    Primitive2DSequence aSequence = parseSvg(u"/svgio/qa/cppunit/data/tdf156251.svg");
+    CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(aSequence.getLength()));
+
+    drawinglayer::Primitive2dXmlDump dumper;
+    xmlDocUniquePtr pDocument = dumper.dumpAndParse(Primitive2DContainer(aSequence));
+
+    CPPUNIT_ASSERT (pDocument);
+
+    // Without the fix in place, this test would have failed with
+    // - Expected: 'You are '
+    // - Actual  : 'You are'
+    assertXPath(pDocument, "/primitive2D/transform/textsimpleportion[1]", "text", "You are ");
+    assertXPath(pDocument, "/primitive2D/transform/textsimpleportion[2]", "text", "not ");
+    assertXPath(pDocument, "/primitive2D/transform/textsimpleportion[3]", "text", "a banana!");
+    assertXPath(pDocument, "/primitive2D/transform/textsimpleportion[4]", "text", "You are ");
+    assertXPath(pDocument, "/primitive2D/transform/textsimpleportion[5]", "text", "not ");
+    assertXPath(pDocument, "/primitive2D/transform/textsimpleportion[6]", "text", "a banana!");
+}
+
 CPPUNIT_TEST_FIXTURE(Test, testMaskText)
 {
     //Check that mask is applied on text
diff --git a/svgio/qa/cppunit/data/tdf156251.svg b/svgio/qa/cppunit/data/tdf156251.svg
new file mode 100644
index 000000000000..201a0aa69e99
--- /dev/null
+++ b/svgio/qa/cppunit/data/tdf156251.svg
@@ -0,0 +1,17 @@
+<svg viewBox="0 0 240 70" xmlns="http://www.w3.org/2000/svg">
+  <style>
+    tspan {
+      fill: red;
+    }
+  </style>
+
+  <text x="10" y="30" class="small">
+    You are
+    <tspan>not</tspan>
+    a banana!
+  </text>
+  <text x="10" y="60" class="small">
+    You are<tspan>   not    </tspan>a banana!
+  </text>
+</svg>
+
diff --git a/svgio/source/svgreader/svgcharacternode.cxx b/svgio/source/svgreader/svgcharacternode.cxx
index 4ca8a3468c02..4ffc46a483db 100644
--- a/svgio/source/svgreader/svgcharacternode.cxx
+++ b/svgio/source/svgreader/svgcharacternode.cxx
@@ -554,7 +554,22 @@ namespace svgio::svgreader
 
         void SvgCharacterNode::whiteSpaceHandling()
         {
-            maText = xmlSpaceHandling(maText, XmlSpace::Default == getXmlSpace());
+            bool bIsDefault(XmlSpace::Default == getXmlSpace());
+            // if xml:space="default" then remove all newline characters, otherwise convert them to space
+            // convert tab to space too
+            maText = maTextBeforeSpaceHandling = maText.replaceAll(u"\n", bIsDefault ? u"" : u" ").replaceAll(u"\t", u" ");
+
+            if(bIsDefault)
+            {
+                // strip of all leading and trailing spaces
+                // and consolidate contiguous space
+                maText = consolidateContiguousSpace(maText.trim());
+            }
+        }
+
+        void SvgCharacterNode::addGap()
+        {
+            maText += " ";
         }
 
         void SvgCharacterNode::concatenate(std::u16string_view rText)
diff --git a/svgio/source/svgreader/svgdocumenthandler.cxx b/svgio/source/svgreader/svgdocumenthandler.cxx
index e0d835700659..7fc95dd8f36d 100644
--- a/svgio/source/svgreader/svgdocumenthandler.cxx
+++ b/svgio/source/svgreader/svgdocumenthandler.cxx
@@ -54,6 +54,7 @@
 #include <svgtitledescnode.hxx>
 #include <sal/log.hxx>
 #include <osl/diagnose.h>
+#include <o3tl/string_view.hxx>
 
 using namespace com::sun::star;
 
@@ -62,7 +63,7 @@ namespace svgio::svgreader
 
 namespace
 {
-    void whiteSpaceHandling(svgio::svgreader::SvgNode const * pNode)
+    svgio::svgreader::SvgCharacterNode* whiteSpaceHandling(svgio::svgreader::SvgNode const * pNode, svgio::svgreader::SvgCharacterNode* pLast)
     {
         if(pNode)
         {
@@ -81,7 +82,47 @@ namespace
                         {
                             // clean whitespace in text span
                             svgio::svgreader::SvgCharacterNode* pCharNode = static_cast< svgio::svgreader::SvgCharacterNode* >(pCandidate);
+
                             pCharNode->whiteSpaceHandling();
+
+                            // pCharNode may have lost all text. If that's the case, ignore
+                            // as invalid character node
+                            // Also ignore if textBeforeSpaceHandling just have spaces
+                            if(!pCharNode->getText().isEmpty() && !o3tl::trim(pCharNode->getTextBeforeSpaceHandling()).empty())
+                            {
+                                if(pLast)
+                                {
+                                    bool bAddGap(true);
+
+                                    // Do not add a gap if last node doesn't end with a space and
+                                    // current note doesn't start with a space
+                                    const sal_uInt32 nLastLength(pLast->getTextBeforeSpaceHandling().getLength());
+                                    if(pLast->getTextBeforeSpaceHandling()[nLastLength - 1] != ' ' && pCharNode->getTextBeforeSpaceHandling()[0] != ' ')
+                                        bAddGap = false;
+
+                                    // With this option a baseline shift between two char parts ('words')
+                                    // will not add a space 'gap' to the end of the (non-last) word. This
+                                    // seems to be the standard behaviour, see last bugdoc attached #122524#
+                                    const svgio::svgreader::SvgStyleAttributes* pStyleLast = pLast->getSvgStyleAttributes();
+                                    const svgio::svgreader::SvgStyleAttributes* pStyleCurrent = pCandidate->getSvgStyleAttributes();
+
+                                    if(pStyleLast && pStyleCurrent && pStyleLast->getBaselineShift() != pStyleCurrent->getBaselineShift())
+                                    {
+                                        bAddGap = false;
+                                    }
+
+                                    // add in-between whitespace (single space) to last
+                                    // known character node
+                                    if(bAddGap)
+                                    {
+                                        pLast->addGap();
+                                    }
+                                }
+
+                                // remember new last corrected character node
+                                pLast = pCharNode;
+                            }
+
                             break;
                         }
                         case SVGToken::Tspan:
@@ -89,7 +130,7 @@ namespace
                         case SVGToken::Tref:
                         {
                             // recursively clean whitespaces in subhierarchy
-                            whiteSpaceHandling(pCandidate);
+                            pLast = whiteSpaceHandling(pCandidate, pLast);
                             break;
                         }
                         default:
@@ -101,8 +142,9 @@ namespace
                 }
             }
         }
-    }
 
+        return pLast;
+    }
 } // end anonymous namespace
 
         SvgDocHdl::SvgDocHdl(const OUString& aAbsolutePath)
@@ -556,7 +598,7 @@ namespace
             if(pWhitespaceCheck)
             {
                 // cleanup read strings
-                whiteSpaceHandling(pWhitespaceCheck);
+                whiteSpaceHandling(pWhitespaceCheck, nullptr);
             }
         }
 
diff --git a/svgio/source/svgreader/svgtools.cxx b/svgio/source/svgreader/svgtools.cxx
index d6d9a5f1604c..9fb930a53467 100644
--- a/svgio/source/svgreader/svgtools.cxx
+++ b/svgio/source/svgreader/svgtools.cxx
@@ -1495,22 +1495,6 @@ namespace svgio::svgreader
             return rCandidate;
         }
 
-        OUString xmlSpaceHandling(const OUString& rCandidate, bool bIsDefault)
-        {
-            // if xml:space="default" then remove all newline characters, otherwise convert them to space
-            // convert tab to space too
-            OUString aRetval(rCandidate.replaceAll(u"\n", bIsDefault ? u"" : u" ").replaceAll(u"\t", u" "));
-
-            if(bIsDefault)
-            {
-                // strip of all leading and trailing spaces
-                // and consolidate contiguous space
-                aRetval = consolidateContiguousSpace(aRetval.trim());
-            }
-
-            return aRetval;
-        }
-
         ::std::vector< double > solveSvgNumberVector(const SvgNumberVector& rInput, const InfoProvider& rInfoProvider)
         {
             ::std::vector< double > aRetval;