tdf#129605 rtfimport: deduplicating borders loses information

Because at least ONE direct attribute still existes, that SPRM
overrides the style SPRM, and therefore the style contents
are ignored. But deduplicating has removed the matching parts
from the paragraph, so any children properties that
match the style revert to default values. In this unit test,
only the green border-color was kept, and so a default
none-border was created.

The UI in writer also directly applies all border properties
during a color change, so further style changes in the
border-style or thickness no longer have any effect.
So there is no reason to try to deduplicate border stuff.

UNRESOLVED PROBLEM NOTE:
If deduplication is going to happen, then there needs to be a
merging of character style child attributes and the directly
applied properties - which in practice is the same as not
deduplicating at all. (The problem will still come when the
paragraph ONLY redefines the border color - then there is
nothing to deduplicate and still the style contents are ignored.)

This patch affects paragraph-style borders (in a good way),
since it fixes tdf#129631 as well. Probably every SPRM
that expects children should just avoid deduplication...

UNRESOLVED PROBLEM NOTE:
Another problem is that the direct properties themselves
do not seem to be deduplicated. In addition, deduplication
happens against the FIRST instance of the property,
not on the LAST instance (which ultimately is the winner).

Change-Id: I97333fba66db5cfb5238de426821fe458def329b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/85868
Tested-by: Jenkins
Reviewed-by: Justin Luth <justin_luth@sil.org>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>

4 files changed, 105 insertions, 0 deletions

diff --git a/sw/qa/extras/rtfexport/data/tdf129522_removeShadowStyle.odt b/sw/qa/extras/rtfexport/data/tdf129522_removeShadowStyle.odt
new file mode 100644
index 000000000000..7ced9fc647c4
--- /dev/null
+++ b/sw/qa/extras/rtfexport/data/tdf129522_removeShadowStyle.odt
diff --git a/sw/qa/extras/rtfexport/data/tdf129631_lostBorders.rtf b/sw/qa/extras/rtfexport/data/tdf129631_lostBorders.rtf
new file mode 100644
index 000000000000..e5b3bc4e4956
--- /dev/null
+++ b/sw/qa/extras/rtfexport/data/tdf129631_lostBorders.rtf
@@ -0,0 +1,27 @@
+{\rtf1\ansi\deff4\adeflang1025
+{\fonttbl{\f0\froman\fprq2\fcharset0 Times New Roman;}{\f1\froman\fprq2\fcharset2 Symbol;}{\f2\fswiss\fprq2\fcharset0 Arial;}{\f3\froman\fprq2\fcharset0 Liberation Serif{\*\falt Times New Roman};}{\f4\froman\fprq0\fcharset0 Times New Roman;}{\f5\fnil\fprq2\fcharset0 DejaVu Sans;}}
+{\colortbl;\red0\green0\blue0;\red0\green0\blue255;\red0\green255\blue255;\red0\green255\blue0;\red255\green0\blue255;\red255\green0\blue0;\red255\green255\blue0;\red255\green255\blue255;\red0\green0\blue128;\red0\green128\blue128;\red0\green128\blue0;\red128\green0\blue128;\red128\green0\blue0;\red128\green128\blue0;\red128\green128\blue128;\red192\green192\blue192;\red6\green154\blue46;}
+{\stylesheet{\s0\snext0\dbch\af5\langfe2052\dbch\af4\afs24\alang1025\ql\widctlpar\ltrpar\hyphpar0\aspalpha\cf0\loch\f3\fs24\lang255\kerning1 Normal;}
+{\*\cs15\snext15 CharShadow;}
+{\*\cs16\sbasedon15\snext16 CharShadow-removed;}
+{\s17\sbasedon0\snext18\dbch\af5\langfe2052\dbch\af4\afs28\alang1025\ql\widctlpar\sb240\sa120\keepn\ltrpar\cf0\loch\f4\fs28\lang255\kerning1 Heading;}
+{\s18\sbasedon0\snext18\dbch\af5\langfe2052\dbch\af4\afs24\alang1025\sl276\slmult1\ql\widctlpar\sb0\sa140\ltrpar\cf0\loch\f3\fs24\lang255\kerning1 Text Body;}
+{\s19\sbasedon18\snext19\dbch\af5\langfe2052\dbch\af4\afs24\alang1025\sl276\slmult1\ql\widctlpar\sb0\sa140\ltrpar\cf0\loch\f4\fs24\lang255\kerning1 List;}
+{\s20\sbasedon0\snext20\dbch\af5\langfe2052\dbch\af4\afs24\alang1025\ai\ql\widctlpar\sb120\sa120\noline\ltrpar\cf0\loch\f4\fs24\lang255\i\kerning1 Caption;}
+{\s21\sbasedon0\snext21\dbch\af5\langfe2052\dbch\af4\afs24\alang1025\ql\widctlpar\noline\ltrpar\cf0\loch\f4\fs24\lang255\kerning1 Index;}
+{\s22\sbasedon0\snext22\dbch\af5\langfe2052\dbch\af4\afs24\alang1025\ql\widctlpar\brdrt\brdrs\brdrw50\brdrcf1\brsp0\brdrl\brdrs\brdrw50\brdrcf1\brsp0\brdrb\brdrs\brdrw50\brdrcf1\brsp20\brdrr\brdrs\brdrw50\brdrcf1\brsp20\ltrpar\cf0\loch\f3\fs48\lang255\i\b\kerning1 Border;}
+{\s23\sbasedon0\snext23\dbch\af5\langfe2052\dbch\af4\afs24\alang1025\ql\widctlpar\sb0\sa60\noline\ltrpar\cf0\loch\f3\fs24\lang255\kerning1 Sender;}
+}{\*\generator LibreOfficeDev/6.5.0.0.alpha0$Linux_X86_64 LibreOffice_project/c71d886120998884fdd16a862826f59883d9a114}{\info{\creatim\yr2019\mo12\dy21\hr9\min10}{\revtim\yr2019\mo12\dy26\hr11\min50}{\printim\yr0\mo0\dy0\hr0\min0}}{\*\userprops}\deftab709
+\viewscale140
+{\*\pgdsctbl
+{\pgdsc0\pgdscuse451\lndscpsxn\pgwsxn8391\pghsxn5953\marglsxn1134\margrsxn1134\margtsxn992\margbsxn992\pgdscnxt0 Default Style;}}
+\formshade{\*\pgdscno0}\landscape\paperh5953\paperw8391\margl1134\margr1134\margt992\margb992\sectd\sbknone\sectunlocked1\lndscpsxn\pgndec\pgwsxn8391\pghsxn5953\marglsxn1134\margrsxn1134\margtsxn992\margbsxn992\ftnbj\ftnstart1\ftnrstcont\ftnnar\aenddoc\aftnrstcont\aftnstart1\aftnnrlc\htmautsp
+{\*\ftnsep\chftnsep}\pgndec\pard\plain \s0\dbch\af5\langfe2052\dbch\af4\afs24\alang1025\ql\widctlpar\ltrpar\hyphpar0\aspalpha\cf0\loch\f3\fs24\lang255\kerning1{\loch
+If a charStyle border is slightly modified by direct formatting, the borders are lost, right?}
+\par \pard\plain \s22\dbch\af5\langfe2052\dbch\af4\afs24\alang1025\ql\widctlpar\cf0\loch\f3\fs48\lang255\i\b\kerning1\brdrt\brdrs\brdrw50\brdrcf17\brsp0\brdrl\brdrs\brdrw50\brdrcf17\brsp0\brdrb\brdrs\brdrw50\brdrcf17\brsp20\brdrr\brdrs\brdrw50\brdrcf17\brsp20{\loch
+This has paragraph border styles, colored green by direct formating.}
+\par \pard\plain \s0\dbch\af5\langfe2052\dbch\af4\afs24\alang1025\ql\widctlpar\ltrpar\hyphpar0\aspalpha\cf0\loch\f3\fs24\lang255\kerning1\loch
+
+\par \pard\plain \s22\dbch\af5\langfe2052\dbch\af4\afs24\alang1025\ql\widctlpar\brdrt\brdrs\brdrw50\brdrcf1\brsp0\brdrl\brdrs\brdrw50\brdrcf1\brsp0\brdrb\brdrs\brdrw50\brdrcf1\brsp20\brdrr\brdrs\brdrw50\brdrcf1\brsp20\ltrpar\cf0\loch\f3\fs48\lang255\i\b\kerning1{\loch
+End of test}
+\par }
diff --git a/sw/qa/extras/rtfexport/rtfexport4.cxx b/sw/qa/extras/rtfexport/rtfexport4.cxx
index f714b0519caf..bbf4c60f6ff2 100644
--- a/sw/qa/extras/rtfexport/rtfexport4.cxx
+++ b/sw/qa/extras/rtfexport/rtfexport4.cxx
@@ -9,6 +9,7 @@
 
 #include <swmodeltestbase.hxx>
 
+#include <com/sun/star/table/ShadowFormat.hpp>
 #include <com/sun/star/text/WritingMode2.hpp>
 #include <com/sun/star/style/ParagraphAdjust.hpp>
 #include <o3tl/cppunittraitshelper.hxx>
@@ -241,6 +242,70 @@ DECLARE_RTFEXPORT_TEST(testBtlrFrame, "btlr-frame.odt")
     CPPUNIT_ASSERT_EQUAL(text::WritingMode2::BT_LR, nActual);
 }
 
+DECLARE_RTFEXPORT_TEST(testTdf129631_lostBorders, "tdf129631_lostBorders.rtf")
+{
+    uno::Reference<container::XNameAccess> paragraphStyles = getStyles("ParagraphStyles");
+    uno::Reference<beans::XPropertySet> xStyleProps(paragraphStyles->getByName("Border"),
+                                                    uno::UNO_QUERY_THROW);
+    table::BorderLine2 aBorderLine = getProperty<table::BorderLine2>(xStyleProps, "RightBorder");
+    CPPUNIT_ASSERT(sal_uInt32(0) != aBorderLine.LineWidth);
+    CPPUNIT_ASSERT_EQUAL_MESSAGE(
+        "The border style has normal black borders", COL_BLACK,
+        Color(getProperty<table::BorderLine>(xStyleProps, "RightBorder").Color));
+
+    aBorderLine = getProperty<table::BorderLine2>(getParagraph(2), "RightBorder");
+    CPPUNIT_ASSERT(sal_uInt32(0) != aBorderLine.LineWidth);
+    CPPUNIT_ASSERT_EQUAL_MESSAGE(
+        "The second paragraph should have dark green borders", sal_Int32(432686),
+        getProperty<table::BorderLine>(getParagraph(2), "RightBorder").Color);
+}
+
+DECLARE_RTFEXPORT_TEST(testTdf129522_removeShadowStyle, "tdf129522_removeShadowStyle.odt")
+{
+    uno::Reference<container::XNameAccess> paragraphStyles = getStyles("ParagraphStyles");
+    uno::Reference<beans::XPropertySet> xStyleProps(paragraphStyles->getByName("Shadow"),
+                                                    uno::UNO_QUERY_THROW);
+    table::ShadowFormat aShadow = getProperty<table::ShadowFormat>(xStyleProps, "ParaShadowFormat");
+    CPPUNIT_ASSERT_EQUAL(table::ShadowLocation_BOTTOM_RIGHT, aShadow.Location);
+
+    // Shadows were inherited regardless of whether the style disabled them.
+    xStyleProps.set(paragraphStyles->getByName("Shadow-removed"), uno::UNO_QUERY_THROW);
+    aShadow = getProperty<table::ShadowFormat>(xStyleProps, "ParaShadowFormat");
+    //CPPUNIT_ASSERT_EQUAL(table::ShadowLocation_NONE, aShadow.Location);
+
+    uno::Reference<container::XNameAccess> characterStyles = getStyles("CharacterStyles");
+    xStyleProps.set(characterStyles->getByName("CharShadow"), uno::UNO_QUERY_THROW);
+    aShadow = getProperty<table::ShadowFormat>(xStyleProps, "CharShadowFormat");
+    CPPUNIT_ASSERT_EQUAL(table::ShadowLocation_BOTTOM_RIGHT, aShadow.Location);
+
+    xStyleProps.set(characterStyles->getByName("CharShadow-removed"), uno::UNO_QUERY_THROW);
+    aShadow = getProperty<table::ShadowFormat>(xStyleProps, "CharShadowFormat");
+    //CPPUNIT_ASSERT_EQUAL(table::ShadowLocation_NONE, aShadow.Location);
+
+    uno::Reference<text::XTextRange> xRun = getRun(getParagraph(1), 2, "style");
+    aShadow = getProperty<table::ShadowFormat>(xRun, "CharShadowFormat");
+    //CPPUNIT_ASSERT_EQUAL(table::ShadowLocation_NONE, aShadow.Location);
+
+    xRun.set(getRun(getParagraph(1), 4, "shadow"));
+    aShadow = getProperty<table::ShadowFormat>(xRun, "CharShadowFormat");
+    CPPUNIT_ASSERT_EQUAL(table::ShadowLocation_BOTTOM_RIGHT, aShadow.Location);
+    table::BorderLine2 aBorderLine = getProperty<table::BorderLine2>(xRun, "CharRightBorder");
+    // MS formats can't have a shadow without a border.
+    // Char borders are all or none, so have to decide to add borders, or throw away shadow...
+    if (mbExported)
+        CPPUNIT_ASSERT(sal_uInt32(0) != aBorderLine.LineWidth);
+
+    xRun.set(getRun(getParagraph(4), 2, "shadow"));
+    aShadow = getProperty<table::ShadowFormat>(xRun, "CharShadowFormat");
+    //CPPUNIT_ASSERT_EQUAL(table::ShadowLocation_NONE, aShadow.Location);
+
+    xRun.set(getRun(getParagraph(9), 2, "End of test"));
+    aShadow = getProperty<table::ShadowFormat>(xRun, "CharShadowFormat");
+    CPPUNIT_ASSERT_EQUAL(table::ShadowLocation_BOTTOM_RIGHT, aShadow.Location);
+    aBorderLine = getProperty<table::BorderLine2>(xRun, "CharRightBorder");
+    CPPUNIT_ASSERT(sal_uInt32(0) != aBorderLine.LineWidth);
+}
+
 CPPUNIT_TEST_FIXTURE(Test, testPageBorder)
 {
     load(mpTestDocumentPath, "page-border.rtf");
diff --git a/writerfilter/source/rtftok/rtfsprm.cxx b/writerfilter/source/rtftok/rtfsprm.cxx
index 76838e3aa620..c7ae29effe2e 100644
--- a/writerfilter/source/rtftok/rtfsprm.cxx
+++ b/writerfilter/source/rtftok/rtfsprm.cxx
@@ -211,6 +211,19 @@ static bool isSPRMDeduplicateBlacklist(Id nId)
         // correct, keep these.
         case NS_ooxml::LN_CT_Spacing_beforeAutospacing:
         case NS_ooxml::LN_CT_Spacing_afterAutospacing:
+        // \chbrdr requires *all* of the border settings to be present,
+        // otherwise a default (NONE) border is created from the removed
+        // attributes which then overrides the style-defined border.
+        // See BorderHandler.cxx and NS_ooxml::LN_EG_RPrBase_bdr in DomainMapper.
+        // This also is needed for NS_ooxml::LN_CT_PBdr_top etc.
+        case NS_ooxml::LN_CT_Border_sz:
+        case NS_ooxml::LN_CT_Border_val:
+        case NS_ooxml::LN_CT_Border_color:
+        case NS_ooxml::LN_CT_Border_space:
+        case NS_ooxml::LN_CT_Border_shadow:
+        case NS_ooxml::LN_CT_Border_frame:
+        case NS_ooxml::LN_CT_Border_themeTint:
+        case NS_ooxml::LN_CT_Border_themeColor:
             return true;
 
         default: