From e49c42d17f50c8b0cac9db08dedc375dd5aa8a98 Mon Sep 17 00:00:00 2001
From: Miklos Vajna <vmiklos@collabora.co.uk>
Date: Wed, 31 Oct 2018 16:38:05 +0100
Subject: oox smartart, vertical bracket list: fix node counter condition

The visible effect of this was that the 2nd level text nodes were
missing from the layout result.

The cause is that when it comes to counting nodes of a condition, we
assumed that the current layout node is a presentation of a model node,
but this is not necessarily true.

Fix the problem doing a "first presentation child of", then a "presentation
of" navigation in that case, which leads us to the correct model node,
so counting its children works.

(An alternative way of getting non-zero children would be a
"presentation parent of" navigation, followed by a "presentation of"
navigation, but that would lead us to the document root, so we would
count the number of 1st level elements, not the correct 2nd level
elements.)

Change-Id: Iccebe0e2e56b7acb7fbe2c38a7c9ebb2abb309b9
Reviewed-on: https://gerrit.libreoffice.org/62703
Reviewed-by: Miklos Vajna <vmiklos@collabora.co.uk>
Tested-by: Jenkins
---
 .../drawingml/diagram/diagramlayoutatoms.cxx       |  48 +++++++++++++++++++--
 sd/qa/unit/data/pptx/vertical-bracket-list.pptx    | Bin 0 -> 42124 bytes
 sd/qa/unit/import-tests-smartart.cxx               |  19 ++++++++
 3 files changed, 64 insertions(+), 3 deletions(-)
 create mode 100644 sd/qa/unit/data/pptx/vertical-bracket-list.pptx

diff --git a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
index 8acceb2a942f..2ef5ffef2151 100644
--- a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
+++ b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
@@ -146,6 +146,36 @@ const dgm::Point* ConditionAtom::getPresNode() const
     return nullptr;
 }
 
+namespace
+{
+/**
+ * Takes the connection list from rLayoutNode, navigates from rFrom on an edge
+ * of type nType, using a direction determined by bSourceToDestination.
+ */
+OUString navigate(const LayoutNode& rLayoutNode, sal_Int32 nType, const OUString& rFrom,
+                  bool bSourceToDestination)
+{
+    for (const auto& rConnection : rLayoutNode.getDiagram().getData()->getConnections())
+    {
+        if (rConnection.mnType != nType)
+            continue;
+
+        if (bSourceToDestination)
+        {
+            if (rConnection.msSourceId == rFrom)
+                return rConnection.msDestId;
+        }
+        else
+        {
+            if (rConnection.msDestId == rFrom)
+                return rConnection.msSourceId;
+        }
+    }
+
+    return OUString();
+}
+}
+
 sal_Int32 ConditionAtom::getNodeCount() const
 {
     sal_Int32 nCount = 0;
@@ -154,9 +184,21 @@ sal_Int32 ConditionAtom::getNodeCount() const
     {
         OUString sNodeId = "";
 
-        for (const auto& aCxn : mrLayoutNode.getDiagram().getData()->getConnections())
-            if (aCxn.mnType == XML_presOf && aCxn.msDestId == pPoint->msModelId)
-                sNodeId = aCxn.msSourceId;
+        sNodeId
+            = navigate(mrLayoutNode, XML_presOf, pPoint->msModelId, /*bSourceToDestination*/ false);
+
+        if (sNodeId.isEmpty())
+        {
+            // The current layout node is not a presentation of anything. Look
+            // up the first presentation child of the layout node.
+            OUString sFirstPresChildId = navigate(mrLayoutNode, XML_presParOf, pPoint->msModelId,
+                                                  /*bSourceToDestination*/ true);
+            if (!sFirstPresChildId.isEmpty())
+                // It has a presentation child: is that a presentation of a
+                // model node?
+                sNodeId = navigate(mrLayoutNode, XML_presOf, sFirstPresChildId,
+                                   /*bSourceToDestination*/ false);
+        }
 
         if (!sNodeId.isEmpty())
         {
diff --git a/sd/qa/unit/data/pptx/vertical-bracket-list.pptx b/sd/qa/unit/data/pptx/vertical-bracket-list.pptx
new file mode 100644
index 000000000000..bef9d7481dfc
Binary files /dev/null and b/sd/qa/unit/data/pptx/vertical-bracket-list.pptx differ
diff --git a/sd/qa/unit/import-tests-smartart.cxx b/sd/qa/unit/import-tests-smartart.cxx
index 3f0d274f2c6a..223b105181d5 100644
--- a/sd/qa/unit/import-tests-smartart.cxx
+++ b/sd/qa/unit/import-tests-smartart.cxx
@@ -41,6 +41,7 @@ public:
     void testSegmentedCycle();
     void testBaseRtoL();
     void testVertialBoxList();
+    void testVertialBracketList();
 
     CPPUNIT_TEST_SUITE(SdImportTestSmartArt);
 
@@ -66,6 +67,7 @@ public:
     CPPUNIT_TEST(testSegmentedCycle);
     CPPUNIT_TEST(testBaseRtoL);
     CPPUNIT_TEST(testVertialBoxList);
+    CPPUNIT_TEST(testVertialBracketList);
 
     CPPUNIT_TEST_SUITE_END();
 };
@@ -397,6 +399,23 @@ void SdImportTestSmartArt::testVertialBoxList()
     xDocShRef->DoClose();
 }
 
+void SdImportTestSmartArt::testVertialBracketList()
+{
+    sd::DrawDocShellRef xDocShRef = loadURL(
+        m_directories.getURLFromSrc("/sd/qa/unit/data/pptx/vertical-bracket-list.pptx"), PPTX);
+    uno::Reference<drawing::XShapes> xShapeGroup(getShapeFromPage(0, 0, xDocShRef),
+                                                 uno::UNO_QUERY_THROW);
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(1), xShapeGroup->getCount());
+
+    uno::Reference<drawing::XShapes> xFirstChild(xShapeGroup->getByIndex(0), uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xFirstChild.is());
+    // Without the accompanying fix in place, this test would have failed with
+    // 'actual: 2', i.e. one child shape (with its "A" text) was missing.
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(3), xFirstChild->getCount());
+
+    xDocShRef->DoClose();
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(SdImportTestSmartArt);
 
 CPPUNIT_PLUGIN_IMPLEMENT();
-- 
cgit v1.2.3