Advise needed for fixing tdf#41253

Hello everyone,

I just have some times to look at tdf#41253.
(https://bugs.documentfoundation.org/41253) But I don't promise I
could finish it, so I still didn't assign myself into a bug. Anyway,
let me explain (and ask) about it:

The bug is about importing pptx file which has shadowed text (one of
the text effects apart from bold, italic, underlined), but LibO can't
import "shadowing text".

The problem is, in OOXML structure, shadowing is declared in
<a:outerShdw> element, which is a child of <a:effectLst>. For more
info, please see http://www.datypic.com/sc/ooxml/e-a_effectLst-1.html

This is the OOXML sample comparing shadowed and non-shadowed text
(cropped from slide1.xml of an extracted pptx file):
---cut---
<a:p>
    <a:r>
        <a:rPr lang="en-US" dirty="0" smtClean="0">
            <a:effectLst>
                <a:outerShdw blurRad="38100" dist="38100"
dir="2700000" algn="tl"><a:srgbClr val="000000"><a:alpha val="43137"/>
                    </a:srgbClr>
                </a:outerShdw>
            </a:effectLst>
        </a:rPr>
        <a:t>Text with shadow</a:t>
    </a:r>
</a:p>
<a:p>
    <a:r>
        <a:rPr lang="en-US" dirty="0" smtClean="0"/>
         <a:t>Text without shadow</a:t>
    </a:r>
    <a:endParaRPr lang="th-TH" dirty="0"/>
</a:p>
---cut---

And the code parsing <a:effectLst> is still unsupported, see:
http://opengrok.libreoffice.org/xref/core/oox/source/drawingml/textcharacterpropertiescontext.cxx#101

This is my patch (also attached to this email). It compiled
successfully, but the shadowing still didn't work:

---cut---
diff --git a/oox/inc/drawingml/textcharacterproperties.hxx
b/oox/inc/drawingml/textcharacterproperties.hxx
index 055e58f..2dca075 100644
--- a/oox/inc/drawingml/textcharacterproperties.hxx
+++ b/oox/inc/drawingml/textcharacterproperties.hxx
@@ -56,6 +56,7 @@ struct TextCharacterProperties
     OptValue< bool >    moUnderlineLineFollowText;
     OptValue< bool >    moUnderlineFillFollowText;
     FillProperties      maFillProperties;
+    OptValue< bool >    moCharShadowed;

     std::vector<css::beans::PropertyValue> maTextEffectsProperties;

diff --git a/oox/source/drawingml/textcharacterproperties.cxx
b/oox/source/drawingml/textcharacterproperties.cxx
index 48803de..c5e1253 100644
--- a/oox/source/drawingml/textcharacterproperties.cxx
+++ b/oox/source/drawingml/textcharacterproperties.cxx
@@ -151,6 +151,10 @@ void TextCharacterProperties::pushToPropMap(
PropertyMap& rPropMap, const XmlFil
         rPropMap.setProperty( PROP_CharUnderlineHasColor, true);
         rPropMap.setProperty( PROP_CharUnderlineColor,
maUnderlineColor.getColor( rFilter.getGraphicHelper() ));
     }
+
+    bool bCharShadowed = moCharShadowed.get(false);
+    SAL_DEBUG(bCharShadowed);
+    rPropMap.setProperty( PROP_CharShadowed, /*true */bCharShadowed);
 }

 void pushToGrabBag( PropertySet& rPropSet, const
std::vector<PropertyValue>& aVectorOfProperyValues )
---cut---

In textcharacterproperties.cxx, it is where the shadowing process
done. If I change the argument to "true", like this:
    rPropMap.setProperty( PROP_CharShadowed, true);
Then all texts will got shadowed. The problem is, bCharShadowed is
never "true" in current patch, and I couldn't get it to be "true".

---cut---
diff --git a/oox/source/drawingml/textcharacterpropertiescontext.cxx
b/oox/source/drawingml/textcharacterpropertiescontext.cxx
index 1c62c1d..ef24ab1 100644
--- a/oox/source/drawingml/textcharacterpropertiescontext.cxx
+++ b/oox/source/drawingml/textcharacterpropertiescontext.cxx
@@ -217,6 +223,10 @@ ContextHandlerRef
TextCharacterPropertiesContext::onCreateContext( sal_Int32 aEl
                 return new TextEffectsContext( *this, aElementToken,
mrTextCharacterProperties.maTextEffectsProperties );
             }
             break;
+        case A_TOKEN(outerShdw):
+            SAL_DEBUG("A_TOKEN now True");
+            mrTextCharacterProperties.moCharShadowed = true;
+            break;
         default:
             SAL_WARN("oox",
"TextCharacterPropertiesContext::onCreateContext: unhandled element: "
<< getBaseToken(aElementToken));
             break;
---cut---

I don't know why bCharShadowed is never "true". But I think this
approach is likely inappropriate because we already have these:

---cut---
        // EG_EffectProperties
        case A_TOKEN( effectDag ):  // CT_EffectContainer 5.1.10.25
        case A_TOKEN( effectLst ):  // CT_EffectList 5.1.10.26
        break;
---cut---

in textcharacterpropertiescontext.cxx line 99-102, which I think it's
more correct to start from these codes.

So, should I start over from the line:
        case A_TOKEN( effectLst ):  // CT_EffectList 5.1.10.26
or any advise about my patch would be much appreciated.


Thank you very much.
Best Regards,
-- 
Korrawit Pruegsanusak

diff --git a/oox/inc/drawingml/textcharacterproperties.hxx 
b/oox/inc/drawingml/textcharacterproperties.hxx
index 055e58f..2dca075 100644
--- a/oox/inc/drawingml/textcharacterproperties.hxx
+++ b/oox/inc/drawingml/textcharacterproperties.hxx
@@ -56,6 +56,7 @@ struct TextCharacterProperties
     OptValue< bool >    moUnderlineLineFollowText;
     OptValue< bool >    moUnderlineFillFollowText;
     FillProperties      maFillProperties;
+    OptValue< bool >    moCharShadowed;
 
     std::vector<css::beans::PropertyValue> maTextEffectsProperties;
 
diff --git a/oox/source/drawingml/textcharacterproperties.cxx 
b/oox/source/drawingml/textcharacterproperties.cxx
index 48803de..c5e1253 100644
--- a/oox/source/drawingml/textcharacterproperties.cxx
+++ b/oox/source/drawingml/textcharacterproperties.cxx
@@ -151,6 +151,10 @@ void TextCharacterProperties::pushToPropMap( PropertyMap& rPropMap, const 
XmlFil
         rPropMap.setProperty( PROP_CharUnderlineHasColor, true);
         rPropMap.setProperty( PROP_CharUnderlineColor, maUnderlineColor.getColor( 
rFilter.getGraphicHelper() ));
     }
+
+    bool bCharShadowed = moCharShadowed.get(false);
+    SAL_DEBUG(bCharShadowed);
+    rPropMap.setProperty( PROP_CharShadowed, /*true */bCharShadowed);
 }
 
 void pushToGrabBag( PropertySet& rPropSet, const std::vector<PropertyValue>& 
aVectorOfProperyValues )
diff --git a/oox/source/drawingml/textcharacterpropertiescontext.cxx 
b/oox/source/drawingml/textcharacterpropertiescontext.cxx
index 1c62c1d..ef24ab1 100644
--- a/oox/source/drawingml/textcharacterpropertiescontext.cxx
+++ b/oox/source/drawingml/textcharacterpropertiescontext.cxx
@@ -217,6 +223,10 @@ ContextHandlerRef TextCharacterPropertiesContext::onCreateContext( sal_Int32 
aEl
                 return new TextEffectsContext( *this, aElementToken, 
mrTextCharacterProperties.maTextEffectsProperties );
             }
             break;
+        case A_TOKEN(outerShdw):
+            SAL_DEBUG("A_TOKEN now True");
+            mrTextCharacterProperties.moCharShadowed = true;
+            break;
         default:
             SAL_WARN("oox", "TextCharacterPropertiesContext::onCreateContext: unhandled element: " 
<< getBaseToken(aElementToken));
             break;