tdf#153791: paragraph's/character's shd overrides shape style's fontRef

I couldn't find any references to this in documentation (ECMA-376, MS-OE376)
regarding this, but Word ignores the font properties (including color) defined
in the shape style's fontRef for txbxContent's paragraphs / runs that have shd
elements with non-auto fill color.

Change-Id: Ice634a5eed7b51379649462303300f55358a566f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147630
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
(cherry picked from commit c15412eb96bda1037c12811f5818ed8ce1e603bd)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147844
Reviewed-by: Christian Lohmaier <lohmaier+LibreOffice@googlemail.com>

3 files changed, 114 insertions, 46 deletions

diff --git a/oox/source/shape/WpsContext.cxx b/oox/source/shape/WpsContext.cxx
index 99656195075b..1d0ecfe95519 100644
--- a/oox/source/shape/WpsContext.cxx
+++ b/oox/source/shape/WpsContext.cxx
@@ -169,62 +169,56 @@ oox::core::ContextHandlerRef WpsContext::onCreateContext(sal_Int32 nElementToken
 
                 // Apply character color of the shape to the shape's textbox.
                 uno::Reference<text::XText> xText(mxShape, uno::UNO_QUERY);
-                uno::Reference<text::XTextCursor> xTextCursor = xText->createTextCursor();
-                xTextCursor->gotoStart(false);
-                xTextCursor->gotoEnd(true);
-                uno::Reference<beans::XPropertySet> xTextBoxPropertySet(xTextCursor,
-                                                                        uno::UNO_QUERY);
                 uno::Any xCharColor = xPropertySet->getPropertyValue("CharColor");
                 Color aColor = COL_AUTO;
                 if ((xCharColor >>= aColor) && aColor != COL_AUTO)
                 {
-                    const uno::Reference<beans::XPropertyState> xPropertyState(xTextCursor,
-                                                                               uno::UNO_QUERY);
-                    const beans::PropertyState ePropertyState
-                        = xPropertyState->getPropertyState("CharColor");
-                    if (ePropertyState == beans::PropertyState_DEFAULT_VALUE)
-                    {
-                        xTextBoxPropertySet->setPropertyValue("CharColor", xCharColor);
-                    }
-                    else
+                    // tdf#135923 Apply character color of the shape to the textrun
+                    //            when the character color of the textrun is default.
+                    // tdf#153791 But only if the run has no background color (shd element in OOXML)
+                    if (uno::Reference<container::XEnumerationAccess> paraEnumAccess{
+                            xText, uno::UNO_QUERY })
                     {
-                        // tdf#135923 Apply character color of the shape to the textrun
-                        //            when the character color of the textrun is default.
-                        uno::Reference<container::XEnumerationAccess> paraEnumAccess(
-                            xText, uno::UNO_QUERY);
-                        if (paraEnumAccess.is())
-                        {
-                            uno::Reference<container::XEnumeration> paraEnum(
-                                paraEnumAccess->createEnumeration());
+                        uno::Reference<container::XEnumeration> paraEnum(
+                            paraEnumAccess->createEnumeration());
 
-                            while (paraEnum->hasMoreElements())
-                            {
-                                uno::Reference<text::XTextRange> xParagraph(paraEnum->nextElement(),
-                                                                            uno::UNO_QUERY);
-                                uno::Reference<container::XEnumerationAccess> runEnumAccess(
-                                    xParagraph, uno::UNO_QUERY);
-                                if (!runEnumAccess.is())
+                        while (paraEnum->hasMoreElements())
+                        {
+                            uno::Reference<text::XTextRange> xParagraph(paraEnum->nextElement(),
+                                                                        uno::UNO_QUERY);
+                            uno::Reference<container::XEnumerationAccess> runEnumAccess(
+                                xParagraph, uno::UNO_QUERY);
+                            if (!runEnumAccess.is())
+                                continue;
+                            if (uno::Reference<beans::XPropertySet> xParaPropSet{ xParagraph,
+                                                                                  uno::UNO_QUERY })
+                                if ((xParaPropSet->getPropertyValue("ParaBackColor") >>= aColor)
+                                    && aColor != COL_AUTO)
                                     continue;
 
-                                uno::Reference<container::XEnumeration> runEnum
-                                    = runEnumAccess->createEnumeration();
+                            uno::Reference<container::XEnumeration> runEnum
+                                = runEnumAccess->createEnumeration();
 
-                                while (runEnum->hasMoreElements())
+                            while (runEnum->hasMoreElements())
+                            {
+                                uno::Reference<text::XTextRange> xRun(runEnum->nextElement(),
+                                                                      uno::UNO_QUERY);
+                                const uno::Reference<beans::XPropertyState> xRunState(
+                                    xRun, uno::UNO_QUERY);
+                                if (!xRunState
+                                    || xRunState->getPropertyState("CharColor")
+                                           == beans::PropertyState_DEFAULT_VALUE)
                                 {
-                                    uno::Reference<text::XTextRange> xRun(runEnum->nextElement(),
-                                                                          uno::UNO_QUERY);
-                                    const uno::Reference<beans::XPropertyState> xRunState(
-                                        xRun, uno::UNO_QUERY);
-                                    if (xRunState->getPropertyState("CharColor")
-                                        == beans::PropertyState_DEFAULT_VALUE)
-                                    {
-                                        uno::Reference<beans::XPropertySet> xRunPropSet(
-                                            xRun, uno::UNO_QUERY);
-                                        Color aRunColor = COL_AUTO;
-                                        xRunPropSet->getPropertyValue("CharColor") >>= aRunColor;
-                                        if (aRunColor == COL_AUTO)
-                                            xRunPropSet->setPropertyValue("CharColor", xCharColor);
-                                    }
+                                    uno::Reference<beans::XPropertySet> xRunPropSet(xRun,
+                                                                                    uno::UNO_QUERY);
+                                    if (!xRunPropSet)
+                                        continue;
+                                    if ((xRunPropSet->getPropertyValue("CharBackColor") >>= aColor)
+                                        && aColor != COL_AUTO)
+                                        continue;
+                                    if (!(xRunPropSet->getPropertyValue("CharColor") >>= aColor)
+                                        || aColor == COL_AUTO)
+                                        xRunPropSet->setPropertyValue("CharColor", xCharColor);
                                 }
                             }
                         }
diff --git a/sw/qa/extras/ooxmlimport/data/tdf153791-shd_overrides_fontRef.docx b/sw/qa/extras/ooxmlimport/data/tdf153791-shd_overrides_fontRef.docx
new file mode 100644
index 000000000000..3706b456ca8e
--- /dev/null
+++ b/sw/qa/extras/ooxmlimport/data/tdf153791-shd_overrides_fontRef.docx
diff --git a/sw/qa/extras/ooxmlimport/ooxmlimport2.cxx b/sw/qa/extras/ooxmlimport/ooxmlimport2.cxx
index a1110d0d287e..26ad31df3a78 100644
--- a/sw/qa/extras/ooxmlimport/ooxmlimport2.cxx
+++ b/sw/qa/extras/ooxmlimport/ooxmlimport2.cxx
@@ -947,6 +947,80 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf152200)
     // Should not crash/hang because of wrong placement of ending fldChar
 }
 
+CPPUNIT_TEST_FIXTURE(Test, testTdf153791)
+{
+    createSwDoc("tdf153791-shd_overrides_fontRef.docx");
+
+    // the first shape (a paragraph with no background)
+    auto xTextBox(getShape(1));
+    CPPUNIT_ASSERT_EQUAL(COL_LIGHTRED, getProperty<Color>(xTextBox, "CharColor"));
+    uno::Reference<text::XTextRange> xRange(xTextBox, uno::UNO_QUERY_THROW);
+    CPPUNIT_ASSERT_EQUAL(OUString("Lorem ipsum"), xRange->getString());
+
+    uno::Reference<container::XEnumerationAccess> xParaEnumAccess(xRange, uno::UNO_QUERY_THROW);
+    uno::Reference<container::XEnumeration> xParaEnum = xParaEnumAccess->createEnumeration();
+
+    uno::Reference<text::XTextRange> xPara(xParaEnum->nextElement(), uno::UNO_QUERY_THROW);
+    CPPUNIT_ASSERT_EQUAL(COL_AUTO, getProperty<Color>(xPara, "ParaBackColor"));
+
+    uno::Reference<container::XEnumerationAccess> xRunEnumAccess(xPara, uno::UNO_QUERY_THROW);
+    uno::Reference<container::XEnumeration> xRunEnum = xRunEnumAccess->createEnumeration();
+
+    uno::Reference<text::XTextRange> xRun(xRunEnum->nextElement(), uno::UNO_QUERY_THROW);
+    CPPUNIT_ASSERT_EQUAL(OUString("Lorem ipsum"), xRun->getString());
+    CPPUNIT_ASSERT_EQUAL(COL_AUTO, getProperty<Color>(xRun, "CharBackColor"));
+    // In the absence of paragraph/character background, the whole paragraph is red.
+    CPPUNIT_ASSERT_EQUAL(COL_LIGHTRED, getProperty<Color>(xRun, "CharColor"));
+
+    // the second shape: two paragraphs
+    xTextBox.set(getShape(2));
+    CPPUNIT_ASSERT_EQUAL(COL_LIGHTRED, getProperty<Color>(xTextBox, "CharColor"));
+    xRange.set(xTextBox, uno::UNO_QUERY_THROW);
+    CPPUNIT_ASSERT_EQUAL(OUString("Lorem ipsum" SAL_NEWLINE_STRING "Lorem ipsum"),
+                         xRange->getString());
+
+    xParaEnumAccess.set(xRange, uno::UNO_QUERY_THROW);
+    xParaEnum = xParaEnumAccess->createEnumeration();
+
+    // the first one has paragraph background
+    xPara.set(xParaEnum->nextElement(), uno::UNO_QUERY_THROW);
+    CPPUNIT_ASSERT_EQUAL(Color(0xF0F0F0), getProperty<Color>(xPara, "ParaBackColor"));
+
+    xRunEnumAccess.set(xPara, uno::UNO_QUERY_THROW);
+    xRunEnum = xRunEnumAccess->createEnumeration();
+
+    xRun.set(xRunEnum->nextElement(), uno::UNO_QUERY_THROW);
+    CPPUNIT_ASSERT_EQUAL(OUString("Lorem ipsum"), xRun->getString());
+    CPPUNIT_ASSERT_EQUAL(COL_AUTO, getProperty<Color>(xRun, "CharBackColor"));
+    // With paragraph background, the whole paragraph is auto.
+    // Without the fix, this would fail with:
+    // - Expected: rgba[ffffff00]
+    // - Actual  : rgba[ff0000ff]
+    CPPUNIT_ASSERT_EQUAL(COL_AUTO, getProperty<Color>(xRun, "CharColor"));
+
+    // the second paragraph has two runs, the last one with character background
+    xPara.set(xParaEnum->nextElement(), uno::UNO_QUERY_THROW);
+    CPPUNIT_ASSERT_EQUAL(COL_AUTO, getProperty<Color>(xPara, "ParaBackColor"));
+
+    xRunEnumAccess.set(xPara, uno::UNO_QUERY_THROW);
+    xRunEnum = xRunEnumAccess->createEnumeration();
+
+    xRun.set(xRunEnum->nextElement(), uno::UNO_QUERY_THROW);
+    CPPUNIT_ASSERT_EQUAL(OUString("Lorem "), xRun->getString());
+    CPPUNIT_ASSERT_EQUAL(COL_AUTO, getProperty<Color>(xRun, "CharBackColor"));
+    // In the absence of paragraph/character background, the run is red
+    CPPUNIT_ASSERT_EQUAL(COL_LIGHTRED, getProperty<Color>(xRun, "CharColor"));
+
+    xRun.set(xRunEnum->nextElement(), uno::UNO_QUERY_THROW);
+    CPPUNIT_ASSERT_EQUAL(OUString("ipsum"), xRun->getString());
+    CPPUNIT_ASSERT_EQUAL(Color(0xF0F0F0), getProperty<Color>(xRun, "CharBackColor"));
+    // With character background, the run is auto.
+    // Without the fix, this would fail with:
+    // - Expected: rgba[ffffff00]
+    // - Actual  : rgba[ff0000ff]
+    CPPUNIT_ASSERT_EQUAL(COL_AUTO, getProperty<Color>(xRun, "CharColor"));
+}
+
 // tests should only be added to ooxmlIMPORT *if* they fail round-tripping in ooxmlEXPORT
 
 CPPUNIT_PLUGIN_IMPLEMENT();