[Libreoffice-commits] core.git: Branch 'distro/collabora/cp-5.3' - sw/qa sw/source
Mike Kaganski
mike.kaganski at collabora.com
Tue Feb 6 17:00:31 UTC 2018
sw/qa/extras/uiwriter/uiwriter.cxx | 58 +++++++++++++++++
sw/source/uibase/wrtsh/delete.cxx | 126 ++++++++++++++++++-------------------
2 files changed, 121 insertions(+), 63 deletions(-)
New commits:
commit e690954bada93bae54e64a306c104da3d540834e
Author: Mike Kaganski <mike.kaganski at collabora.com>
Date: Mon Feb 5 22:27:14 2018 +0300
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 80a4b3b589a516392bcf1ad932619701eb95e250 is
not intuitive, and differs from what other word processors do.
Unit test included.
Change-Id: Icb53b6733f0d7394abe011fa067089e6693cf648
Reviewed-on: https://gerrit.libreoffice.org/49257
Tested-by: Jenkins <ci at libreoffice.org>
Reviewed-by: Mike Kaganski <mike.kaganski at collabora.com>
Reviewed-on: https://gerrit.libreoffice.org/49292
Reviewed-by: Aron Budea <aron.budea at collabora.com>
Tested-by: Aron Budea <aron.budea at collabora.com>
diff --git a/sw/qa/extras/uiwriter/uiwriter.cxx b/sw/qa/extras/uiwriter/uiwriter.cxx
index 95206bf7944d..bf98a565b1f2 100644
--- a/sw/qa/extras/uiwriter/uiwriter.cxx
+++ b/sw/qa/extras/uiwriter/uiwriter.cxx
@@ -242,6 +242,7 @@ public:
void testCreateDocxAnnotation();
void testTdf113790();
void testTdf115013();
+ void testTdf115132();
CPPUNIT_TEST_SUITE(SwUiWriterTest);
CPPUNIT_TEST(testReplaceForward);
@@ -371,6 +372,7 @@ public:
CPPUNIT_TEST(testCreateDocxAnnotation);
CPPUNIT_TEST(testTdf113790);
CPPUNIT_TEST(testTdf115013);
+ CPPUNIT_TEST(testTdf115132);
CPPUNIT_TEST_SUITE_END();
private:
@@ -4696,6 +4698,62 @@ void SwUiWriterTest::testTdf115013()
utl::removeTree(aWorkDir);
}
+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();
diff --git a/sw/source/uibase/wrtsh/delete.cxx b/sw/source/uibase/wrtsh/delete.cxx
index a300ad2ba059..411c7e079df7 100644
--- a/sw/source/uibase/wrtsh/delete.cxx
+++ b/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()
{
@@ -176,6 +177,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 ?
@@ -184,23 +189,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(false);
return 0;
+ }
+
+ SwCursorShell::Pop(true);
OpenMark();
SwCursorShell::Right(1,CRSR_SKIP_CHARS);
@@ -241,8 +257,6 @@ long SwWrtShell::DelRight()
if(nSelection & nsSelectionType::SEL_TXT)
nSelection = nsSelectionType::SEL_TXT;
- const SwTableNode * pWasInTableNd = nullptr;
-
switch( nSelection & ~(nsSelectionType::SEL_BEZ) )
{
case nsSelectionType::SEL_POSTIT:
@@ -277,70 +291,56 @@ long SwWrtShell::DelRight()
EnterStdMode();
}
- pWasInTableNd = IsCursorInTable();
-
- if( nsSelectionType::SEL_TXT & 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 = nsSelectionType::SEL_TXT & 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( false );
- 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( false );
- }
-
// If we are just ahead of a fieldmark, then remove it completely
sw::mark::IFieldmark* pFm = GetCurrentFieldmark();
if (pFm && pFm->GetMarkStart() == *GetCursor()->GetPoint())
More information about the Libreoffice-commits
mailing list