SmartArt: correct behaviour of if/else nodes

else block is taken only if none of conditions in 'choose' was satisfied

Change-Id: Ie668f15c665327098e8e63b2c92cd291711e4567
Reviewed-on: https://gerrit.libreoffice.org/41533
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Jan Holesovsky <kendy@collabora.com>

3 files changed, 35 insertions, 44 deletions

diff --git a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
index dd69d19c1f5f..920648706fa0 100644
--- a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
+++ b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
@@ -98,9 +98,20 @@ void ChooseAtom::accept( LayoutAtomVisitor& rVisitor )
     rVisitor.visit(*this);
 }
 
-ConditionAtom::ConditionAtom(const LayoutNode& rLayoutNode, const Reference< XFastAttributeList >& xAttributes) :
+const std::vector<LayoutAtomPtr>& ChooseAtom::getChildren() const
+{
+    for (const auto& pChild : mpChildNodes)
+    {
+        const ConditionAtomPtr pCond = std::dynamic_pointer_cast<ConditionAtom>(pChild);
+        if (pCond && pCond->getDecision())
+            return pCond->getChildren();
+    }
+    return maEmptyChildren;
+}
+
+ConditionAtom::ConditionAtom(const LayoutNode& rLayoutNode, bool isElse, const Reference< XFastAttributeList >& xAttributes) :
     LayoutAtom(rLayoutNode),
-    mbElse( false )
+    mIsElse(isElse)
 {
     maIter.loadFromXAttr( xAttributes );
     maCond.loadFromXAttr( xAttributes );
@@ -156,36 +167,33 @@ sal_Int32 ConditionAtom::getNodeCount() const
     return nCount;
 }
 
-const std::vector<LayoutAtomPtr>& ConditionAtom::getChildren() const
+bool ConditionAtom::getDecision() const
 {
-    bool bDecisionVar = true;
+    if (mIsElse)
+        return true;
+
     switch (maCond.mnFunc)
     {
     case XML_var:
     {
         const dgm::Point* pPoint = getPresNode();
         if (pPoint && maCond.mnArg == XML_dir)
-            bDecisionVar = compareResult(maCond.mnOp, pPoint->mnDirection, maCond.mnVal);
+            return compareResult(maCond.mnOp, pPoint->mnDirection, maCond.mnVal);
         break;
     }
 
     case XML_cnt:
-        bDecisionVar = compareResult(maCond.mnOp, getNodeCount(), maCond.msVal.toInt32());
-        break;
+        return compareResult(maCond.mnOp, getNodeCount(), maCond.msVal.toInt32());
 
     case XML_maxDepth:
-        bDecisionVar = compareResult(maCond.mnOp, mrLayoutNode.getDiagram().getData()->getMaxDepth(), maCond.msVal.toInt32());
-        break;
+        return compareResult(maCond.mnOp, mrLayoutNode.getDiagram().getData()->getMaxDepth(), maCond.msVal.toInt32());
 
     default:
         SAL_WARN("oox.drawingml", "unknown function " << maCond.mnFunc);
         break;
     }
 
-    if (bDecisionVar)
-        return mpChildNodes;
-    else
-        return mpElseChildNodes;
+    return true;
 }
 
 void ConditionAtom::accept( LayoutAtomVisitor& rVisitor )
@@ -193,14 +201,6 @@ void ConditionAtom::accept( LayoutAtomVisitor& rVisitor )
     rVisitor.visit(*this);
 }
 
-void ConditionAtom::addChild( const LayoutAtomPtr & pNode )
-{
-    if( mbElse )
-        mpElseChildNodes.push_back( pNode );
-    else
-        mpChildNodes.push_back( pNode );
-}
-
 void ConstraintAtom::accept( LayoutAtomVisitor& rVisitor )
 {
     rVisitor.visit(*this);
diff --git a/oox/source/drawingml/diagram/diagramlayoutatoms.hxx b/oox/source/drawingml/diagram/diagramlayoutatoms.hxx
index f4928565c048..117326b625ab 100644
--- a/oox/source/drawingml/diagram/diagramlayoutatoms.hxx
+++ b/oox/source/drawingml/diagram/diagramlayoutatoms.hxx
@@ -176,21 +176,17 @@ class ConditionAtom
     : public LayoutAtom
 {
 public:
-    explicit ConditionAtom(const LayoutNode& rLayoutNode, const css::uno::Reference< css::xml::sax::XFastAttributeList >& xAttributes);
+    explicit ConditionAtom(const LayoutNode& rLayoutNode, bool isElse, const css::uno::Reference< css::xml::sax::XFastAttributeList >& xAttributes);
     virtual void accept( LayoutAtomVisitor& ) override;
-    void readElseBranch()
-        { mbElse=true; }
-    virtual void addChild( const LayoutAtomPtr & pNode ) override;
-    virtual const std::vector<LayoutAtomPtr>& getChildren() const override;
+    bool getDecision() const;
 private:
     static bool compareResult(sal_Int32 nOperator, sal_Int32 nFirst, sal_Int32 nSecond);
     const dgm::Point* getPresNode() const;
     sal_Int32 getNodeCount() const;
 
-    bool          mbElse;
+    bool          mIsElse;
     IteratorAttr  maIter;
     ConditionAttr maCond;
-    std::vector< LayoutAtomPtr > mpElseChildNodes;
 };
 
 typedef std::shared_ptr< ConditionAtom > ConditionAtomPtr;
@@ -202,6 +198,9 @@ class ChooseAtom
 public:
     ChooseAtom(const LayoutNode& rLayoutNode) : LayoutAtom(rLayoutNode) {}
     virtual void accept( LayoutAtomVisitor& ) override;
+    virtual const std::vector<LayoutAtomPtr>& getChildren() const override;
+private:
+    const std::vector<LayoutAtomPtr> maEmptyChildren;
 };
 
 class LayoutNode
diff --git a/oox/source/drawingml/diagram/layoutnodecontext.cxx b/oox/source/drawingml/diagram/layoutnodecontext.cxx
index 1d36aeccebb1..586e9e103421 100644
--- a/oox/source/drawingml/diagram/layoutnodecontext.cxx
+++ b/oox/source/drawingml/diagram/layoutnodecontext.cxx
@@ -104,24 +104,17 @@ public:
             case DGM_TOKEN( if ):
             {
                 // CT_When
-                mpConditionNode.reset( new ConditionAtom(mpNode->getLayoutNode(), rAttribs.getFastAttributeList()) );
-                mpNode->addChild( mpConditionNode );
-                return new IfContext( *this, rAttribs, mpConditionNode );
+                ConditionAtomPtr pNode( new ConditionAtom(mpNode->getLayoutNode(), false, rAttribs.getFastAttributeList()) );
+                mpNode->addChild( pNode );
+                return new IfContext( *this, rAttribs, pNode );
             }
             case DGM_TOKEN( else ):
+            {
                 // CT_Otherwise
-                if( mpConditionNode )
-                {
-                    mpConditionNode->readElseBranch();
-                    ContextHandlerRef xRet = new IfContext( *this, rAttribs, mpConditionNode );
-                    mpConditionNode.reset();
-                    return xRet;
-                }
-                else
-                {
-                    SAL_WARN("oox",  "ignoring second else clause" );
-                }
-                break;
+                ConditionAtomPtr pNode( new ConditionAtom(mpNode->getLayoutNode(), true, rAttribs.getFastAttributeList()) );
+                mpNode->addChild( pNode );
+                return new IfContext( *this, rAttribs, pNode );
+            }
             default:
                 break;
             }
@@ -131,7 +124,6 @@ public:
 private:
     OUString msName;
     LayoutAtomPtr mpNode;
-    ConditionAtomPtr mpConditionNode;
 };
 
 class ForEachContext