tdf#115132: don't move out from current cell on Backspace/Delete

The behavior that cursor jumps out of current table box on Del/Backspace
introduced in commit 80a4b3b5 is
not intuitive, and differs from what other word processors do.

Unit test included.

Change-Id: Icb53b6733f0d7394abe011fa067089e6693cf648
Reviewed-on: https://gerrit.libreoffice.org/49257Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

sw/qa/extras/uiwriter/uiwriter.cxx

@@ -300,6 +300,7 @@ public:
     void testTdf115013();
     void testTdf114536();
     void testTdf115065();
+    void testTdf115132();
 
     CPPUNIT_TEST_SUITE(SwUiWriterTest);
     CPPUNIT_TEST(testReplaceForward);
@@ -479,6 +480,7 @@ public:
     CPPUNIT_TEST(testTdf115013);
     CPPUNIT_TEST(testTdf114536);
     CPPUNIT_TEST(testTdf115065);
+    CPPUNIT_TEST(testTdf115132);
     CPPUNIT_TEST_SUITE_END();
 
 private:
@@ -5885,6 +5887,62 @@ void SwUiWriterTest::testTdf115065()
     pWrtShell->Copy(pWrtShell, ptFrom, ptTo);
 }
 
+void SwUiWriterTest::testTdf115132()
+{
+    SwDoc* pDoc = createDoc();
+    CPPUNIT_ASSERT(pDoc);
+    SwWrtShell* pWrtShell = pDoc->GetDocShell()->GetWrtShell();
+    CPPUNIT_ASSERT(pWrtShell);
+
+    std::vector<OUString> vTestTableNames;
+
+    // Create an empty paragraph that will separate first table from the rest
+    pWrtShell->SplitNode();
+    pWrtShell->SttDoc();
+    // Create a table at the start of document body
+    SwInsertTableOptions TableOpt(tabopts::DEFAULT_BORDER, 0);
+    const SwTable* pTable = &pWrtShell->InsertTable(TableOpt, 2, 3);
+    const SwTableFormat* pFormat = pTable->GetFrameFormat();
+    CPPUNIT_ASSERT(pFormat);
+    vTestTableNames.push_back(pFormat->GetName());
+    pWrtShell->EndDoc();
+    // Create a table after a paragraph
+    pTable = &pWrtShell->InsertTable(TableOpt, 2, 3);
+    pFormat = pTable->GetFrameFormat();
+    CPPUNIT_ASSERT(pFormat);
+    vTestTableNames.push_back(pFormat->GetName());
+    // Create a table immediately after the previous
+    pTable = &pWrtShell->InsertTable(TableOpt, 2, 3);
+    pFormat = pTable->GetFrameFormat();
+    CPPUNIT_ASSERT(pFormat);
+    vTestTableNames.push_back(pFormat->GetName());
+    // Create a nested table in the middle of last row
+    pWrtShell->GotoTable(vTestTableNames.back());
+    for (int i = 0; i < 4; ++i)
+        pWrtShell->GoNextCell(false);
+    pTable = &pWrtShell->InsertTable(TableOpt, 2, 3);
+    pFormat = pTable->GetFrameFormat();
+    CPPUNIT_ASSERT(pFormat);
+    vTestTableNames.push_back(pFormat->GetName());
+
+    // Now check that in any cell in all tables we don't go out of a cell
+    // using Delete or Backspace. We test cases when a table is the first node;
+    // when we are in a first/middle/last cell in a row; when there's a paragraph
+    // before/after this cell; when there's another table before/after this cell;
+    // in nested table.
+    for (const auto& rTableName : vTestTableNames)
+    {
+        pWrtShell->GotoTable(rTableName);
+        do {
+            const SwStartNode* pNd = pWrtShell->GetSwCursor()->GetNode().FindTableBoxStartNode();
+            pWrtShell->DelRight();
+            CPPUNIT_ASSERT_EQUAL(pNd, pWrtShell->GetSwCursor()->GetNode().FindTableBoxStartNode());
+            pWrtShell->DelLeft();
+            CPPUNIT_ASSERT_EQUAL(pNd, pWrtShell->GetSwCursor()->GetNode().FindTableBoxStartNode());
+        } while (pWrtShell->GoNextCell(false));
+    }
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(SwUiWriterTest);
 CPPUNIT_PLUGIN_IMPLEMENT();
 

sw/source/uibase/wrtsh/delete.cxx

@@ -22,6 +22,7 @@
 #include <editeng/lrspitem.hxx>
 #include <view.hxx>
 #include <drawbase.hxx>
+#include <unobaseclass.hxx>
 
 inline void SwWrtShell::OpenMark()
 {
@@ -195,6 +196,10 @@ long SwWrtShell::DelLeft()
 
     if( SwCursorShell::IsSttPara())
     {
+        // Start/EndAllAction to avoid cursor flickering
+        UnoActionContext c(GetDoc());
+        SwCursorShell::Push();
+
         // #i4032# Don't actually call a 'delete' if we
         // changed the table cell, compare DelRight().
         const SwStartNode * pSNdOld = pWasInTableNd ?
@@ -203,23 +208,34 @@ long SwWrtShell::DelLeft()
 
         // If the cursor is at the beginning of a paragraph, try to step
         // backwards. On failure we are done.
-        if( !SwCursorShell::Left(1,CRSR_SKIP_CHARS) )
-            return 0;
+        bool bDoSomething = SwCursorShell::Left(1,CRSR_SKIP_CHARS);
 
-        // If the cursor entered or left a table (or both) we are done. No step
-        // back.
-        const SwTableNode* pIsInTableNd = SwCursorShell::IsCursorInTable();
-        if( pIsInTableNd != pWasInTableNd )
-            return 0;
+        if (bDoSomething)
+        {
+            // If the cursor entered or left a table (or both) we are done.
+            const SwTableNode* pIsInTableNd = SwCursorShell::IsCursorInTable();
+            bDoSomething = pIsInTableNd == pWasInTableNd;
 
-        const SwStartNode* pSNdNew = pIsInTableNd ?
-                                     GetSwCursor()->GetNode().FindTableBoxStartNode() :
-                                     nullptr;
+            if (bDoSomething)
+            {
+                const SwStartNode* pSNdNew = pIsInTableNd ?
+                    GetSwCursor()->GetNode().FindTableBoxStartNode() :
+                    nullptr;
 
-        // #i4032# Don't actually call a 'delete' if we
-        // changed the table cell, compare DelRight().
-        if ( pSNdOld != pSNdNew )
+                // #i4032# Don't actually call a 'delete' if we
+                // changed the table cell, compare DelRight().
+                bDoSomething = pSNdOld == pSNdNew;
+            }
+        }
+
+        if (!bDoSomething)
+        {
+            // tdf#115132 Restore previous position and we are done
+            SwCursorShell::Pop(SwCursorShell::PopMode::DeleteCurrent);
             return 0;
+        }
+
+        SwCursorShell::Pop(SwCursorShell::PopMode::DeleteStack);
 
         OpenMark();
         SwCursorShell::Right(1,CRSR_SKIP_CHARS);
@@ -283,8 +299,6 @@ long SwWrtShell::DelRight()
     if(nSelection & SelectionType::Text)
         nSelection = SelectionType::Text;
 
-    const SwTableNode * pWasInTableNd = nullptr;
-
     switch( nSelection & ~SelectionType::Ornament )
     {
     case SelectionType::PostIt:
@@ -319,70 +333,56 @@ long SwWrtShell::DelRight()
                 EnterStdMode();
         }
 
-        pWasInTableNd = IsCursorInTable();
-
-        if( SelectionType::Text & nSelection && SwCursorShell::IsSttPara() &&
-            SwCursorShell::IsEndPara() )
+        if (SwCursorShell::IsEndPara())
         {
-            // save cursor
-            SwCursorShell::Push();
+            // Start/EndAllAction to avoid cursor flickering
+            UnoActionContext c(GetDoc());
 
+            const SwTableNode* pWasInTableNd = IsCursorInTable();
+            // #108049# Save the startnode of the current cell
+            const SwStartNode* pSNdOld = pWasInTableNd ?
+                GetSwCursor()->GetNode().FindTableBoxStartNode() : nullptr;
+            bool bCheckDelFull = SelectionType::Text & nSelection && SwCursorShell::IsSttPara();
             bool bDelFull = false;
-            if ( SwCursorShell::Right(1,CRSR_SKIP_CHARS) )
+            bool bDoNothing = false;
+
+            // #i41424# Introduced a couple of
+            // Push()-Pop() pairs here. The reason for this is that a
+            // Right()-Left() combination does not make sure, that
+            // the cursor will be in its initial state, because there
+            // may be a numbering in front of the next paragraph.
+            SwCursorShell::Push();
+
+            if (SwCursorShell::Right(1, CRSR_SKIP_CHARS))
             {
-                const SwTableNode * pCurrTableNd = IsCursorInTable();
-                bDelFull = pCurrTableNd && pCurrTableNd != pWasInTableNd;
+                const SwTableNode* pCurrTableNd = IsCursorInTable();
+                bDelFull = bCheckDelFull && pCurrTableNd && pCurrTableNd != pWasInTableNd;
+                if (!bDelFull && (IsCursorInTable() || (pCurrTableNd != pWasInTableNd)))
+                {
+                    // #108049# Save the startnode of the current cell.
+                    // May be different to pSNdOld as we have moved.
+                    const SwStartNode* pSNdNew = pCurrTableNd ?
+                        GetSwCursor()->GetNode().FindTableBoxStartNode() : nullptr;
+
+                    // tdf#115132 Only keep cursor position instead of deleting
+                    // if we have moved to a different cell
+                    bDoNothing = pSNdOld != pSNdNew;
+                }
             }
 
             // restore cursor
             SwCursorShell::Pop(SwCursorShell::PopMode::DeleteCurrent);
 
-            if( bDelFull )
+            if (bDelFull)
             {
                 DelFullPara();
                 UpdateAttr();
-                break;
             }
+            if (bDelFull || bDoNothing)
+                break;
         }
 
         {
-            // #108049# Save the startnode of the current cell
-            const SwStartNode * pSNdOld;
-            pSNdOld = GetSwCursor()->GetNode().FindTableBoxStartNode();
-
-            if ( SwCursorShell::IsEndPara() )
-            {
-                // #i41424# Introduced a couple of
-                // Push()-Pop() pairs here. The reason for this is that a
-                // Right()-Left() combination does not make sure, that
-                // the cursor will be in its initial state, because there
-                // may be a numbering in front of the next paragraph.
-                SwCursorShell::Push();
-
-                if ( SwCursorShell::Right(1, CRSR_SKIP_CHARS) )
-                {
-                    if (IsCursorInTable() || (pWasInTableNd != IsCursorInTable()))
-                    {
-                        /** #108049# Save the startnode of the current
-                            cell. May be different to pSNdOld as we have
-                            moved. */
-                        const SwStartNode * pSNdNew = GetSwCursor()
-                            ->GetNode().FindTableBoxStartNode();
-
-                        /** #108049# Only move instead of deleting if we
-                            have moved to a different cell */
-                        if (pSNdOld != pSNdNew)
-                        {
-                            SwCursorShell::Pop();
-                            break;
-                        }
-                    }
-                }
-
-                // restore cursor
-                SwCursorShell::Pop(SwCursorShell::PopMode::DeleteCurrent);
-            }
-
             // If we are just ahead of a fieldmark, then remove it completely
             sw::mark::IFieldmark* pFm = GetCurrentFieldmark();
             if (pFm && pFm->GetMarkStart() == *GetCursor()->GetPoint())