[Libreoffice-commits] core.git: oox/source sd/qa

[Libreoffice Gerrit user]

 oox/source/drawingml/diagram/diagramlayoutatoms.cxx |   48 ++++++++++++++++++--
 sd/qa/unit/data/pptx/vertical-bracket-list.pptx     |binary
 sd/qa/unit/import-tests-smartart.cxx                |   19 +++++++
 3 files changed, 64 insertions(+), 3 deletions(-)

New commits:
commit e49c42d17f50c8b0cac9db08dedc375dd5aa8a98
Author:     Miklos Vajna <vmiklos at collabora.co.uk>
AuthorDate: Wed Oct 31 16:38:05 2018 +0100
Commit:     Miklos Vajna <vmiklos at collabora.co.uk>
CommitDate: Thu Nov 1 07:36:08 2018 +0100

    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 at collabora.co.uk>
    Tested-by: Jenkins

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();