[Libreoffice-commits] core.git: editeng/source include/editeng sd/qa
Ashod Nakashian (via logerrit)
logerrit at kemper.freedesktop.org
Sun Mar 15 17:50:44 UTC 2020
editeng/source/editeng/editeng.cxx | 9 -
editeng/source/editeng/editview.cxx | 2
editeng/source/editeng/impedit3.cxx | 6
editeng/source/outliner/outliner.cxx | 7 -
include/editeng/editeng.hxx | 6
sd/qa/unit/tiledrendering/tiledrendering.cxx | 185 ++++++++++++++++++++++++++-
6 files changed, 203 insertions(+), 12 deletions(-)
New commits:
commit 8c3af6240d5765fb8954025226477670e1d2a7c7
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
AuthorDate: Sun Mar 1 14:08:20 2020 -0500
Commit: Ashod Nakashian <ashnakash at gmail.com>
CommitDate: Sun Mar 15 18:50:08 2020 +0100
editeng: lok: send cursor visibility event when restoring update mode
When the default text is removed from a TextBox within a slide,
the cursor visibility is inadvertendly set to false and never
restored (because the LOK notification is disabled due to treating
the ShowCursor during SetUpdateMode as an activation of the TextBox,
and that is supressed to avoid messing up the cursor when creating
a new view).
We add a new flag to SetUpdateMode to flag whether this is an
activation or we are restoring a previously active window (TextBox)
due to a temporary disabling (to clear the default text).
Four unit-tests added not just to check and validate the fix,
but to also simulate two different ways of entering edit mode,
first by single-clicking on the text and then double-clicking
outside the text, but within the TextBox.
Change-Id: Icaaabc2a897f614f5ce162b71fadccff22ecda02
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/90301
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
Reviewed-by: Miklos Vajna <vmiklos at collabora.com>
(cherry picked from commit 6b84dfabbb5f6930f9ac582f8c1dd9f467fd068c)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/90298
Tested-by: Jenkins
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
diff --git a/editeng/source/editeng/editeng.cxx b/editeng/source/editeng/editeng.cxx
index 9871e502316f..5a6aee022a80 100644
--- a/editeng/source/editeng/editeng.cxx
+++ b/editeng/source/editeng/editeng.cxx
@@ -1449,11 +1449,14 @@ sal_uInt32 EditEngine::CalcTextWidth()
return nWidth;
}
-void EditEngine::SetUpdateMode( bool bUpdate )
+void EditEngine::SetUpdateMode(bool bUpdate, bool bRestoring)
{
pImpEditEngine->SetUpdateMode( bUpdate );
- if ( pImpEditEngine->pActiveView )
- pImpEditEngine->pActiveView->ShowCursor( false, false, /*bActivate=*/true );
+ if (pImpEditEngine->pActiveView)
+ {
+ // Not an activation if we are restoring the previous update mode.
+ pImpEditEngine->pActiveView->ShowCursor(false, false, /*bActivate=*/!bRestoring);
+ }
}
bool EditEngine::GetUpdateMode() const
diff --git a/editeng/source/editeng/editview.cxx b/editeng/source/editeng/editview.cxx
index 315f9ef6716f..3848553cf4d9 100644
--- a/editeng/source/editeng/editview.cxx
+++ b/editeng/source/editeng/editview.cxx
@@ -482,7 +482,7 @@ void EditView::ShowCursor( bool bGotoCursor, bool bForceVisCursor, bool bActivat
if (pParent && pParent->GetLOKWindowId() != 0)
return;
- OString aPayload = OString::boolean(true);
+ static const OString aPayload = OString::boolean(true);
pImpEditView->mpViewShell->libreOfficeKitViewCallback(LOK_CALLBACK_CURSOR_VISIBLE, aPayload.getStr());
pImpEditView->mpViewShell->NotifyOtherViews(LOK_CALLBACK_VIEW_CURSOR_VISIBLE, "visible", aPayload);
}
diff --git a/editeng/source/editeng/impedit3.cxx b/editeng/source/editeng/impedit3.cxx
index 6ec2a6152ee6..05ab3ccf6823 100644
--- a/editeng/source/editeng/impedit3.cxx
+++ b/editeng/source/editeng/impedit3.cxx
@@ -3939,12 +3939,12 @@ EditPaM ImpEditEngine::ConnectContents( sal_Int32 nLeftNode, bool bBackward )
void ImpEditEngine::SetUpdateMode( bool bUp, EditView* pCurView, bool bForceUpdate )
{
- bool bChanged = ( GetUpdateMode() != bUp );
+ const bool bChanged = (GetUpdateMode() != bUp);
- // When switching from sal_True to sal_False, all selections were visible,
+ // When switching from true to false, all selections were visible,
// => paint over
// the other hand, were all invisible => paint
- // If !bFormatted, e.g. after SetText, then if UpdateMode=sal_True
+ // If !bFormatted, e.g. after SetText, then if UpdateMode=true
// formatting is not needed immediately, probably because more text is coming.
// At latest it is formatted at a Paint/CalcTextWidth.
bUpdate = bUp;
diff --git a/editeng/source/outliner/outliner.cxx b/editeng/source/outliner/outliner.cxx
index 072851d2702f..f7f8cc316ad1 100644
--- a/editeng/source/outliner/outliner.cxx
+++ b/editeng/source/outliner/outliner.cxx
@@ -399,7 +399,7 @@ void Outliner::SetText( const OUString& rText, Paragraph* pPara )
{
DBG_ASSERT(pPara,"SetText:No Para");
- sal_Int32 nPara = pParaList->GetAbsPos( pPara );
+ const sal_Int32 nPara = pParaList->GetAbsPos( pPara );
if (pEditEngine->GetText( nPara ) == rText)
{
@@ -408,7 +408,7 @@ void Outliner::SetText( const OUString& rText, Paragraph* pPara )
return;
}
- bool bUpdate = pEditEngine->GetUpdateMode();
+ const bool bUpdate = pEditEngine->GetUpdateMode();
pEditEngine->SetUpdateMode( false );
ImplBlockInsertionCallbacks( true );
@@ -482,7 +482,8 @@ void Outliner::SetText( const OUString& rText, Paragraph* pPara )
DBG_ASSERT(pParaList->GetParagraphCount()==pEditEngine->GetParagraphCount(),"SetText failed!");
bFirstParaIsEmpty = false;
ImplBlockInsertionCallbacks( false );
- pEditEngine->SetUpdateMode( bUpdate );
+ // Restore the update mode.
+ pEditEngine->SetUpdateMode(bUpdate, /*bRestoring=*/true);
}
// pView == 0 -> Ignore tabs
diff --git a/include/editeng/editeng.hxx b/include/editeng/editeng.hxx
index 5f45dd1fbf29..20c15e6539e1 100644
--- a/include/editeng/editeng.hxx
+++ b/include/editeng/editeng.hxx
@@ -209,7 +209,11 @@ public:
void SetRefMapMode( const MapMode& rMapMode );
MapMode const & GetRefMapMode() const;
- void SetUpdateMode( bool bUpdate );
+ /// Change the update mode per bUpdate and potentially trigger FormatAndUpdate.
+ /// bRestoring is used for LOK to update cursor visibility, specifically,
+ /// when true, it means we are restoring the update mode after internally
+ /// disabling it (f.e. during SetText to set/delete default text in Impress).
+ void SetUpdateMode(bool bUpdate, bool bRestoring = false);
bool GetUpdateMode() const;
void SetBackgroundColor( const Color& rColor );
diff --git a/sd/qa/unit/tiledrendering/tiledrendering.cxx b/sd/qa/unit/tiledrendering/tiledrendering.cxx
index 3f374b0fa051..2fed5db352df 100644
--- a/sd/qa/unit/tiledrendering/tiledrendering.cxx
+++ b/sd/qa/unit/tiledrendering/tiledrendering.cxx
@@ -98,6 +98,10 @@ public:
void testViewCursors();
void testViewCursorParts();
void testCursorViews();
+ void testCursorVisibility_SingleClick();
+ void testCursorVisibility_DoubleClick();
+ void testCursorVisibility_MultiView();
+ void testCursorVisibility_Escape();
void testViewLock();
void testUndoLimiting();
void testCreateViewGraphicSelection();
@@ -149,6 +153,10 @@ public:
CPPUNIT_TEST(testViewCursors);
CPPUNIT_TEST(testViewCursorParts);
CPPUNIT_TEST(testCursorViews);
+ CPPUNIT_TEST(testCursorVisibility_SingleClick);
+ CPPUNIT_TEST(testCursorVisibility_DoubleClick);
+ CPPUNIT_TEST(testCursorVisibility_MultiView);
+ CPPUNIT_TEST(testCursorVisibility_Escape);
CPPUNIT_TEST(testViewLock);
CPPUNIT_TEST(testUndoLimiting);
CPPUNIT_TEST(testCreateViewGraphicSelection);
@@ -958,6 +966,7 @@ public:
/// Our current part, to be able to decide if a view cursor/selection is relevant for us.
int m_nPart;
bool m_bCursorVisibleChanged;
+ bool m_bCursorVisible;
bool m_bViewLock;
bool m_bTilesInvalidated;
std::vector<tools::Rectangle> m_aInvalidations;
@@ -971,6 +980,7 @@ public:
m_bGraphicViewSelectionInvalidated(false),
m_nPart(0),
m_bCursorVisibleChanged(false),
+ m_bCursorVisible(false),
m_bViewLock(false),
m_bTilesInvalidated(false),
m_bViewSelectionSet(false)
@@ -1030,6 +1040,7 @@ public:
case LOK_CALLBACK_CURSOR_VISIBLE:
{
m_bCursorVisibleChanged = true;
+ m_bCursorVisible = (OString("true") == pPayload);
}
break;
case LOK_CALLBACK_VIEW_LOCK:
@@ -1054,7 +1065,7 @@ public:
std::stringstream aStream(pPayload);
boost::property_tree::ptree aTree;
boost::property_tree::read_json(aStream, aTree);
- int nViewId = aTree.get_child("viewId").get_value<int>();
+ const int nViewId = aTree.get_child("viewId").get_value<int>();
m_aViewCursorVisibilities[nViewId] = OString("true") == pPayload;
}
break;
@@ -1173,6 +1184,178 @@ void SdTiledRenderingTest::testCursorViews()
CPPUNIT_ASSERT(aView2.m_bTilesInvalidated);
}
+void SdTiledRenderingTest::testCursorVisibility_SingleClick()
+{
+ // Single-clicking in a text box enters editing only
+ // when it's on the text, even if it's the default text.
+
+ // Load doc.
+ SdXImpressDocument* pXImpressDocument = createDoc("dummy.odp");
+ ViewCallback aView1;
+
+ // Begin text edit on the only object on the slide.
+ sd::ViewShell* pViewShell = pXImpressDocument->GetDocShell()->GetViewShell();
+ SdPage* pActualPage = pViewShell->GetActualPage();
+ SdrObject* pObject1 = pActualPage->GetObj(0);
+ CPPUNIT_ASSERT(pObject1 != nullptr);
+ CPPUNIT_ASSERT_EQUAL(static_cast<sal_uInt16>(OBJ_TITLETEXT), pObject1->GetObjIdentifier());
+ SdrTextObj* pTextObject = static_cast<SdrTextObj*>(pObject1);
+
+ // Click once outside of the text (in the first quartile) => no editing.
+ const ::tools::Rectangle aRect = pTextObject->GetCurrentBoundRect();
+ const auto cornerX = convertMm100ToTwip(aRect.getX() + (aRect.getWidth() / 4));
+ const auto cornerY = convertMm100ToTwip(aRect.getY() + (aRect.getHeight() / 4));
+ pXImpressDocument->postMouseEvent(LOK_MOUSEEVENT_MOUSEBUTTONDOWN,
+ cornerX, cornerY,
+ 1, MOUSE_LEFT, 0);
+ pXImpressDocument->postMouseEvent(LOK_MOUSEEVENT_MOUSEBUTTONUP,
+ cornerX, cornerY,
+ 1, MOUSE_LEFT, 0);
+ Scheduler::ProcessEventsToIdle();
+
+ // No editing.
+ CPPUNIT_ASSERT(!pViewShell->GetView()->IsTextEdit());
+ CPPUNIT_ASSERT(!aView1.m_bCursorVisible);
+
+ // Click again, now on the text, in the center, to start editing.
+ const auto centerX = convertMm100ToTwip(aRect.getX() + (aRect.getWidth() / 2));
+ const auto centerY = convertMm100ToTwip(aRect.getY() + (aRect.getHeight() / 2));
+ pXImpressDocument->postMouseEvent(LOK_MOUSEEVENT_MOUSEBUTTONDOWN,
+ centerX, centerY,
+ 1, MOUSE_LEFT, 0);
+ pXImpressDocument->postMouseEvent(LOK_MOUSEEVENT_MOUSEBUTTONUP,
+ centerX, centerY,
+ 1, MOUSE_LEFT, 0);
+ Scheduler::ProcessEventsToIdle();
+
+ // We must be in text editing mode and have cursor visible.
+ CPPUNIT_ASSERT(pViewShell->GetView()->IsTextEdit());
+ CPPUNIT_ASSERT(aView1.m_bCursorVisible);
+}
+
+
+void SdTiledRenderingTest::testCursorVisibility_DoubleClick()
+{
+ // Double-clicking anywhere in the TextBox should start editing.
+
+ // Create the first view.
+ SdXImpressDocument* pXImpressDocument = createDoc("dummy.odp");
+ ViewCallback aView1;
+
+ // Begin text edit on the only object on the slide.
+ sd::ViewShell* pViewShell = pXImpressDocument->GetDocShell()->GetViewShell();
+ SdPage* pActualPage = pViewShell->GetActualPage();
+ SdrObject* pObject1 = pActualPage->GetObj(0);
+ CPPUNIT_ASSERT(pObject1 != nullptr);
+ CPPUNIT_ASSERT_EQUAL(static_cast<sal_uInt16>(OBJ_TITLETEXT), pObject1->GetObjIdentifier());
+ SdrTextObj* pTextObject = static_cast<SdrTextObj*>(pObject1);
+
+ // Double-click outside the text to enter edit mode.
+ const ::tools::Rectangle aRect = pTextObject->GetCurrentBoundRect();
+ const auto cornerX = convertMm100ToTwip(aRect.getX() + (aRect.getWidth() / 4));
+ const auto cornerY = convertMm100ToTwip(aRect.getY() + (aRect.getHeight() / 4));
+ pXImpressDocument->postMouseEvent(LOK_MOUSEEVENT_MOUSEBUTTONDOWN,
+ cornerX, cornerY,
+ 2, MOUSE_LEFT, 0);
+ pXImpressDocument->postMouseEvent(LOK_MOUSEEVENT_MOUSEBUTTONUP,
+ cornerX, cornerY,
+ 2, MOUSE_LEFT, 0);
+ Scheduler::ProcessEventsToIdle();
+
+ // We must be in text editing mode and have cursor visible.
+ CPPUNIT_ASSERT(pViewShell->GetView()->IsTextEdit());
+ CPPUNIT_ASSERT(aView1.m_bCursorVisible);
+}
+
+void SdTiledRenderingTest::testCursorVisibility_MultiView()
+{
+ // Create the first view.
+ SdXImpressDocument* pXImpressDocument = createDoc("dummy.odp");
+ const int nView1 = SfxLokHelper::getView();
+ ViewCallback aView1;
+
+ // Begin text edit on the only object on the slide.
+ sd::ViewShell* pViewShell = pXImpressDocument->GetDocShell()->GetViewShell();
+ SdPage* pActualPage = pViewShell->GetActualPage();
+ SdrObject* pObject1 = pActualPage->GetObj(0);
+ CPPUNIT_ASSERT(pObject1);
+ CPPUNIT_ASSERT_EQUAL(static_cast<sal_uInt16>(OBJ_TITLETEXT), pObject1->GetObjIdentifier());
+ SdrTextObj* pTextObject = static_cast<SdrTextObj*>(pObject1);
+
+ // Make sure that cursor state is not changed just because we create a second view.
+ SfxLokHelper::createView();
+ pXImpressDocument->initializeForTiledRendering(uno::Sequence<beans::PropertyValue>());
+ const int nView2 = SfxLokHelper::getView();
+ Scheduler::ProcessEventsToIdle();
+ CPPUNIT_ASSERT_EQUAL(false, aView1.m_bCursorVisibleChanged);
+ CPPUNIT_ASSERT_EQUAL(false, aView1.m_aViewCursorVisibilities[nView2]);
+
+ // Also check that the second view gets the notifications.
+ ViewCallback aView2;
+
+ SfxLokHelper::setView(nView1);
+
+ ::tools::Rectangle aRect = pTextObject->GetCurrentBoundRect();
+ const auto centerX = convertMm100ToTwip(aRect.getX() + (aRect.getWidth() / 2));
+ const auto centerY = convertMm100ToTwip(aRect.getY() + (aRect.getHeight() / 2));
+ pXImpressDocument->postMouseEvent(LOK_MOUSEEVENT_MOUSEBUTTONDOWN,
+ centerX, centerY,
+ 2, MOUSE_LEFT, 0);
+ pXImpressDocument->postMouseEvent(LOK_MOUSEEVENT_MOUSEBUTTONUP,
+ centerX, centerY,
+ 2, MOUSE_LEFT, 0);
+ Scheduler::ProcessEventsToIdle();
+
+ // We must be in text editing mode and have cursor visible.
+ CPPUNIT_ASSERT(pViewShell->GetView()->IsTextEdit());
+ CPPUNIT_ASSERT(aView1.m_bCursorVisible);
+ CPPUNIT_ASSERT_EQUAL(false, aView1.m_aViewCursorVisibilities[nView2]);
+
+ CPPUNIT_ASSERT_EQUAL(false, aView2.m_bCursorVisible);
+ CPPUNIT_ASSERT_EQUAL(false, aView2.m_aViewCursorVisibilities[nView1]);
+ CPPUNIT_ASSERT_EQUAL(false, aView2.m_aViewCursorVisibilities[nView2]);
+}
+
+void SdTiledRenderingTest::testCursorVisibility_Escape()
+{
+ // Load doc.
+ SdXImpressDocument* pXImpressDocument = createDoc("dummy.odp");
+ ViewCallback aView1;
+
+ // Begin text edit on the only object on the slide.
+ sd::ViewShell* pViewShell = pXImpressDocument->GetDocShell()->GetViewShell();
+ SdPage* pActualPage = pViewShell->GetActualPage();
+ SdrObject* pObject1 = pActualPage->GetObj(0);
+ CPPUNIT_ASSERT(pObject1 != nullptr);
+ CPPUNIT_ASSERT_EQUAL(static_cast<sal_uInt16>(OBJ_TITLETEXT), pObject1->GetObjIdentifier());
+ SdrTextObj* pTextObject = static_cast<SdrTextObj*>(pObject1);
+
+ // Click once on the text to start editing.
+ const ::tools::Rectangle aRect = pTextObject->GetCurrentBoundRect();
+ const auto centerX = convertMm100ToTwip(aRect.getX() + (aRect.getWidth() / 2));
+ const auto centerY = convertMm100ToTwip(aRect.getY() + (aRect.getHeight() / 2));
+ pXImpressDocument->postMouseEvent(LOK_MOUSEEVENT_MOUSEBUTTONDOWN,
+ centerX, centerY,
+ 1, MOUSE_LEFT, 0);
+ pXImpressDocument->postMouseEvent(LOK_MOUSEEVENT_MOUSEBUTTONUP,
+ centerX, centerY,
+ 1, MOUSE_LEFT, 0);
+ Scheduler::ProcessEventsToIdle();
+
+ // We must be in text editing mode and have cursor visible.
+ CPPUNIT_ASSERT(pViewShell->GetView()->IsTextEdit());
+ CPPUNIT_ASSERT(aView1.m_bCursorVisible);
+
+ // End editing by pressing the escape key.
+ pXImpressDocument->postKeyEvent(LOK_KEYEVENT_KEYINPUT, 0, awt::Key::ESCAPE);
+ pXImpressDocument->postKeyEvent(LOK_KEYEVENT_KEYUP, 0, awt::Key::ESCAPE);
+ Scheduler::ProcessEventsToIdle();
+
+ // We must be in text editing mode and have cursor visible.
+ CPPUNIT_ASSERT(!pViewShell->GetView()->IsTextEdit());
+ CPPUNIT_ASSERT_EQUAL(false, aView1.m_bCursorVisible);
+}
+
void SdTiledRenderingTest::testViewLock()
{
// Load a document that has a shape and create two views.
More information about the Libreoffice-commits
mailing list