bnc#584721: invisible text because of wrong color (white)

Color::getColor() method uses some caching mechanism which
works wrong when the result depend on one of the input parameters.
So avoid caching in these cases.

(cherry picked from commit cfe658c289de030dc3a8fecd3bac0a0004a18061)

Conflicts:
sd/qa/unit/import-tests.cxx

Change-Id: Ifa9221e21e685715454de86d5cec09ff6c266307
Reviewed-on: https://gerrit.libreoffice.org/11723
Reviewed-by: Andras Timar <andras.timar@collabora.com>
Tested-by: Andras Timar <andras.timar@collabora.com>

3 files changed, 61 insertions, 10 deletions

diff --git a/oox/source/drawingml/color.cxx b/oox/source/drawingml/color.cxx
index 52d5465d46a3..ec084cc1bb5f 100644
--- a/oox/source/drawingml/color.cxx
+++ b/oox/source/drawingml/color.cxx
@@ -342,13 +342,10 @@ void Color::clearTransparence()
 
 sal_Int32 Color::getColor( const GraphicHelper& rGraphicHelper, sal_Int32 nPhClr ) const
 {
-    /*  Special handling for theme style list placeholder colors (state
-        COLOR_PH), Color::getColor() may be called with different placeholder
-        colors in the nPhClr parameter. Therefore, the resolved color will not
-        be stored in this object, thus the state COLOR_FINAL will not be
-        reached and the transformation container will not be cleared, but the
-        original COLOR_PH state will be restored instead. */
-    bool bIsPh = false;
+    const sal_Int32 nTempC1 = mnC1;
+    const sal_Int32 nTempC2 = mnC2;
+    const sal_Int32 nTempC3 = mnC3;
+    const ColorMode eTempMode = meMode;
 
     switch( meMode )
     {
@@ -361,7 +358,7 @@ sal_Int32 Color::getColor( const GraphicHelper& rGraphicHelper, sal_Int32 nPhClr
         case COLOR_SCHEME:  setResolvedRgb( rGraphicHelper.getSchemeColor( mnC1 ) );        break;
         case COLOR_PALETTE: setResolvedRgb( rGraphicHelper.getPaletteColor( mnC1 ) );       break;
         case COLOR_SYSTEM:  setResolvedRgb( rGraphicHelper.getSystemColor( mnC1, mnC2 ) );  break;
-        case COLOR_PH:      setResolvedRgb( nPhClr ); bIsPh = true;                         break;
+        case COLOR_PH:      setResolvedRgb( nPhClr );                                       break;
 
         case COLOR_FINAL:   return mnC1;
     }
@@ -497,10 +494,23 @@ sal_Int32 Color::getColor( const GraphicHelper& rGraphicHelper, sal_Int32 nPhClr
         mnC1 = API_RGB_TRANSPARENT;
     }
 
-    meMode = bIsPh ? COLOR_PH : COLOR_FINAL;
+    sal_Int32 nRet = mnC1;
+    // Restore the original values when the color depends on one of the input
+    // parameters (rGraphicHelper or nPhClr)
+    if( eTempMode >= COLOR_SCHEME && eTempMode <= COLOR_PH )
+    {
+        mnC1 = nTempC1;
+        mnC2 = nTempC2;
+        mnC3 = nTempC3;
+        meMode = eTempMode;
+    }
+    else
+    {
+        meMode = COLOR_FINAL;
+    }
     if( meMode == COLOR_FINAL )
         maTransforms.clear();
-    return mnC1;
+    return nRet;
 }
 
 bool Color::hasTransparency() const
diff --git a/sd/qa/unit/data/pptx/bnc584721_4.pptx b/sd/qa/unit/data/pptx/bnc584721_4.pptx
new file mode 100644
index 000000000000..102ee0b14785
--- /dev/null
+++ b/sd/qa/unit/data/pptx/bnc584721_4.pptx
diff --git a/sd/qa/unit/import-tests.cxx b/sd/qa/unit/import-tests.cxx
index d931c10f2324..df848de1c28a 100644
--- a/sd/qa/unit/import-tests.cxx
+++ b/sd/qa/unit/import-tests.cxx
@@ -69,6 +69,7 @@ public:
     void testBnc862510_5();
     void testBnc480256();
     void testCreationDate();
+    void testBnc584721_4();
 
     CPPUNIT_TEST_SUITE(SdFiltersTest);
     CPPUNIT_TEST(testDocumentLayout);
@@ -87,6 +88,7 @@ public:
     CPPUNIT_TEST(testBnc862510_5);
     CPPUNIT_TEST(testBnc480256);
     CPPUNIT_TEST(testCreationDate);
+    CPPUNIT_TEST(testBnc584721_4);
 
     CPPUNIT_TEST_SUITE_END();
 };
@@ -692,6 +694,45 @@ void SdFiltersTest::testBnc480256()
     xDocShRef->DoClose();
 }
 
+void SdFiltersTest::testBnc584721_4()
+{
+    // Black text was imported as white because of wrong caching mechanism
+
+    ::sd::DrawDocShellRef xDocShRef = loadURL(getURLFromSrc("/sd/qa/unit/data/pptx/bnc584721_4.pptx"));
+
+    uno::Reference< drawing::XDrawPagesSupplier > xDoc(
+        xDocShRef->GetDoc()->getUnoModel(), uno::UNO_QUERY_THROW );
+
+    uno::Reference< drawing::XDrawPage > xPage(
+        xDoc->getDrawPages()->getByIndex(1), uno::UNO_QUERY_THROW );
+
+    uno::Reference< beans::XPropertySet > xShape(
+        xPage->getByIndex(1), uno::UNO_QUERY );
+    CPPUNIT_ASSERT_MESSAGE( "no text shape", xShape.is() );
+
+    // Get first paragraph of the text
+    uno::Reference<text::XText> xText = uno::Reference<text::XTextRange>(xShape, uno::UNO_QUERY)->getText();
+    CPPUNIT_ASSERT_MESSAGE( "no text shape", xText.is() );
+    uno::Reference<container::XEnumerationAccess> paraEnumAccess;
+    paraEnumAccess.set(xText, uno::UNO_QUERY);
+    uno::Reference<container::XEnumeration> paraEnum = paraEnumAccess->createEnumeration();
+    uno::Reference<text::XTextRange> const xParagraph(paraEnum->nextElement(),
+                uno::UNO_QUERY_THROW);
+
+    // Get first run of the paragraph
+    uno::Reference<container::XEnumerationAccess> xRunEnumAccess(xParagraph, uno::UNO_QUERY);
+    uno::Reference<container::XEnumeration> xRunEnum = xRunEnumAccess->createEnumeration();
+    uno::Reference<text::XTextRange> xRun(xRunEnum->nextElement(), uno::UNO_QUERY);
+    uno::Reference< beans::XPropertySet > xPropSet( xRun, uno::UNO_QUERY_THROW );
+    sal_Int32 nCharColor;
+    xPropSet->getPropertyValue( "CharColor" ) >>= nCharColor;
+
+    // Color should be black
+    CPPUNIT_ASSERT_EQUAL( sal_Int32(COL_BLACK), nCharColor );
+
+    xDocShRef->DoClose();
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(SdFiltersTest);
 
 CPPUNIT_PLUGIN_IMPLEMENT();