[Libreoffice-commits] core.git: sw/qa writerfilter/source

Justin Luth (via logerrit) logerrit at kemper.freedesktop.org
Thu Jan 2 09:29:09 UTC 2020


 sw/qa/extras/rtfexport/data/tdf129522_removeShadowStyle.odt |binary
 sw/qa/extras/rtfexport/data/tdf129631_lostBorders.rtf       |   27 ++++
 sw/qa/extras/rtfexport/rtfexport4.cxx                       |   65 ++++++++++++
 writerfilter/source/rtftok/rtfsprm.cxx                      |   13 ++
 4 files changed, 105 insertions(+)

New commits:
commit 83832ea3890d9418f17b480ececa204ae54cee18
Author:     Justin Luth <justin.luth at collabora.com>
AuthorDate: Thu Dec 26 11:17:21 2019 +0300
Commit:     Miklos Vajna <vmiklos at collabora.com>
CommitDate: Thu Jan 2 10:28:37 2020 +0100

    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 at sil.org>
    Reviewed-by: Miklos Vajna <vmiklos at collabora.com>

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
Binary files /dev/null and b/sw/qa/extras/rtfexport/data/tdf129522_removeShadowStyle.odt differ
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:


More information about the Libreoffice-commits mailing list