From effd65a586468321dcf0aeab34872d5607102477 Mon Sep 17 00:00:00 2001 From: Juergen Spitzmueller Date: Thu, 24 Aug 2023 09:56:09 +0200 Subject: [PATCH] Amend 101363352c111 Instead of just disabling MOUSE_MOTION on inset buttons, we now register the coordinates where the latest mouse press happens, and if that's on the inset button we are on when releasing, we toggle the inset. Thus, minimal unintentional mouse movements don't lead to the button not to respond. --- src/Cursor.cpp | 8 ++++++++ src/Cursor.h | 9 +++++++++ src/Text.cpp | 3 +++ src/insets/Inset.cpp | 25 +++++++------------------ src/insets/InsetCollapsible.cpp | 10 +++++++--- 5 files changed, 34 insertions(+), 21 deletions(-) diff --git a/src/Cursor.cpp b/src/Cursor.cpp index b20ab0fc0e..18b0e7f50a 100644 --- a/src/Cursor.cpp +++ b/src/Cursor.cpp @@ -714,6 +714,7 @@ bool CursorData::confirmDeletion(bool const before) const Cursor::Cursor(BufferView & bv) : CursorData(&bv.buffer()), bv_(&bv), x_target_(-1), textTargetOffset_(0), + x_clickpos_(-1), y_clickpos_(-1), beforeDispatchPosX_(0), beforeDispatchPosY_(0) {} @@ -1381,6 +1382,13 @@ void Cursor::updateTextTargetOffset() } +void Cursor::setClickPos(int x, int y) +{ + x_clickpos_ = x; + y_clickpos_ = y; +} + + bool Cursor::selHandle(bool selecting) { //lyxerr << "Cursor::selHandle" << endl; diff --git a/src/Cursor.h b/src/Cursor.h index f549047985..50a633ba98 100644 --- a/src/Cursor.h +++ b/src/Cursor.h @@ -380,6 +380,12 @@ public: void setTargetX(); /// clear targetX, i.e. set it to -1 void clearTargetX(); + /// return x position of latest mouse press or -1 if unset + int xClickPos() const { return x_clickpos_; } + /// return y position of latest mouse press or -1 if unset + int yClickPos() const { return y_clickpos_; } + /// register mouse press coordinates + void setClickPos(int x, int y); /// set offset to actual position - targetX void updateTextTargetOffset(); /// distance between actual and targeted position during last up/down in text @@ -469,6 +475,9 @@ private: int x_target_; /// if a x_target cannot be hit exactly in a text, put the difference here int textTargetOffset_; + /// Exact position of mouse click + int x_clickpos_; + int y_clickpos_; /// position before dispatch started DocIterator beforeDispatchCursor_; /// cursor screen coordinates before dispatch started diff --git a/src/Text.cpp b/src/Text.cpp index 9e5bf5ff75..3a8be769e4 100644 --- a/src/Text.cpp +++ b/src/Text.cpp @@ -5223,6 +5223,7 @@ void Text::dispatch(Cursor & cur, FuncRequest & cmd) bvcur.setMark(false); switch (cmd.button()) { case mouse_button::button1: + bvcur.setClickPos(cmd.x(), cmd.y()); if (!bvcur.selection()) // Set the cursor bvcur.resetAnchor(); @@ -5314,6 +5315,8 @@ void Text::dispatch(Cursor & cur, FuncRequest & cmd) case LFUN_MOUSE_RELEASE: switch (cmd.button()) { case mouse_button::button1: + // unregister last mouse press position + cur.bv().cursor().setClickPos(-1, -1); // Cursor was set at LFUN_MOUSE_PRESS or LFUN_MOUSE_MOTION time. // If there is a new selection, update persistent selection; // otherwise, single click does not clear persistent selection diff --git a/src/insets/Inset.cpp b/src/insets/Inset.cpp index 05c6250a35..6e89950f96 100644 --- a/src/insets/Inset.cpp +++ b/src/insets/Inset.cpp @@ -383,26 +383,15 @@ bool Inset::showInsetDialog(BufferView * bv) const void Inset::doDispatch(Cursor & cur, FuncRequest &cmd) { switch (cmd.action()) { - // FIXME: The LFUN_MOUSE_MOTION case is a potential fix for #12418, and maybe also - // #12820 and #12279. This needs to be tested in the pre-release. Also it might - // add slight regressions with inset selection (when selection starts on the button - // of an inset). - // After this has been tested (by Mac users primarily), this comment should be - // updated if the fix is kept or after it has been modified. - case LFUN_MOUSE_MOTION: - // Do not attempt to select while hovering the inset button only (#12418). - if (!cur.selection() && cmd.button() == mouse_button::button1 - && clickable(cur.bv(), cmd.x(), cmd.y())) { - cur.noScreenUpdate(); - cur.dispatched(); - } else - cur.undispatched(); - break; case LFUN_MOUSE_RELEASE: - // if the derived inset did not explicitly handle mouse_release, - // we assume we request the settings dialog - if (!cur.selection() && cmd.button() == mouse_button::button1 + // If the derived inset did not explicitly handle mouse_release, + // we assume we request the settings dialog, + // except if we are about to select (MOUSE_MOTION that started + // outside the inset). + if ((!cur.selection() || covers(cur.bv(), cur.bv().cursor().xClickPos(), + cur.bv().cursor().yClickPos())) + && cmd.button() == mouse_button::button1 && clickable(cur.bv(), cmd.x(), cmd.y()) && hasSettings()) { FuncRequest tmpcmd(LFUN_INSET_SETTINGS); dispatch(cur, tmpcmd); diff --git a/src/insets/InsetCollapsible.cpp b/src/insets/InsetCollapsible.cpp index eb04ca8527..7633af1050 100644 --- a/src/insets/InsetCollapsible.cpp +++ b/src/insets/InsetCollapsible.cpp @@ -504,6 +504,8 @@ docstring const InsetCollapsible::getNewLabel(docstring const & l) const void InsetCollapsible::edit(Cursor & cur, bool front, EntryDirection entry_from) { //lyxerr << "InsetCollapsible: edit left/right" << endl; + // We might have a selection if we moved the mouse on the button only + cur.clearSelection(); cur.push(*this); InsetText::edit(cur, front, entry_from); } @@ -577,9 +579,11 @@ void InsetCollapsible::doDispatch(Cursor & cur, FuncRequest & cmd) cur.noScreenUpdate(); break; } - // if we are selecting, we do not want to - // toggle the inset. - if (cur.selection()) + // If we are selecting, we do not want to toggle the inset + // except if the selection started at the inset button we're still on. + // The latter addresses #12820. + if (cur.selection() && !clickable(cur.bv(), cur.bv().cursor().xClickPos(), + cur.bv().cursor().yClickPos())) break; // Left button is clicked, the user asks to // toggle the inset visual state.