editeng/source/editeng/editview.cxx | 63 +++++++++++++++++++++++---------- editeng/source/misc/urlfieldhelper.cxx | 4 +- include/editeng/editview.hxx | 10 +++-- include/editeng/urlfieldhelper.hxx | 6 +-- sc/source/ui/drawfunc/drtxtob.cxx | 4 +- sw/source/uibase/shells/drwtxtex.cxx | 2 - 6 files changed, 59 insertions(+), 30 deletions(-)
New commits: commit f2a5d091ba9f01a26139e6fc2f3c2bcfd0a6cf3b Author: Justin Luth <[email protected]> AuthorDate: Thu Nov 2 14:42:07 2023 -0400 Commit: Justin Luth <[email protected]> CommitDate: Wed Nov 15 20:03:42 2023 +0100 tdf#158031 editeng SID_*_HYPERLINK: use AlsoCheckBeforeCursor This fixes the GTK3 problem where only the first half of the link successfully ran SID_EDIT_HYPERLINK against an editeng hyperlink. Fixed for Writer and Calc editeng hyperlinks (i.e. in textboxes). TODO: fix SID_COPY_HYPERLINK (which doesn't SelectFieldAtCursor) The problem showed up when the menu option was added because the mouse was over an unselected field, but when the mouse moved to select the menu option, the slot was not considered valid anymore if the cursor had moved behind the field. (The right-click to open the context menu puts the cursor before or after the field.) Now, by checking for a field-before-the-cursor during validation, the field is found and the command is not prevented from running. Once this is all in place, some older commits from Caolan should be able to be reverted. (Strangely, SID_OPEN_HYPERLINK and SID_REMOVE_HYPERLINK seem to run before the slot is invalidated. No idea why they are different. For GEN, the popup usually either kept the pre-menu mouse position, or else it always runs the command before the slot invalidates.) Change-Id: If992b3fdc94d89162b60447458cabced2d5e3f19 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/158856 Reviewed-by: Justin Luth <[email protected]> Tested-by: Jenkins diff --git a/editeng/source/editeng/editview.cxx b/editeng/source/editeng/editview.cxx index bfa98d1ae702..e1229b6918db 100644 --- a/editeng/source/editeng/editview.cxx +++ b/editeng/source/editeng/editview.cxx @@ -1424,11 +1424,11 @@ void EditView::SelectFieldAtCursor() assert(std::abs(aSel.nStartPos - aSel.nEndPos) == 1); } -const SvxFieldData* EditView::GetFieldAtCursor() const +const SvxFieldData* EditView::GetFieldAtCursor(bool bAlsoCheckBeforeCursor) const { const SvxFieldItem* pFieldItem = GetFieldUnderMousePointer(); if (!pFieldItem) - pFieldItem = GetFieldAtSelection(); + pFieldItem = GetFieldAtSelection(bAlsoCheckBeforeCursor); return pFieldItem ? pFieldItem->GetField() : nullptr; } diff --git a/editeng/source/misc/urlfieldhelper.cxx b/editeng/source/misc/urlfieldhelper.cxx index 564bc54e781e..1ed8647a15ae 100644 --- a/editeng/source/misc/urlfieldhelper.cxx +++ b/editeng/source/misc/urlfieldhelper.cxx @@ -25,7 +25,7 @@ void URLFieldHelper::RemoveURLField(EditView& pEditView) } } -bool URLFieldHelper::IsCursorAtURLField(const EditView& pEditView) +bool URLFieldHelper::IsCursorAtURLField(const EditView& pEditView, bool bAlsoCheckBeforeCursor) { // tdf#128666 Make sure only URL field (or nothing) is selected ESelection aSel = pEditView.GetSelection(); @@ -37,7 +37,7 @@ bool URLFieldHelper::IsCursorAtURLField(const EditView& pEditView) if (!bIsValidSelection) return false; - const SvxFieldData* pField = pEditView.GetFieldAtCursor(); + const SvxFieldData* pField = pEditView.GetFieldAtCursor(bAlsoCheckBeforeCursor); if (dynamic_cast<const SvxURLField*>(pField)) return true; diff --git a/include/editeng/editview.hxx b/include/editeng/editview.hxx index 4c18ac2ba3e4..b54f6b463e1f 100644 --- a/include/editeng/editview.hxx +++ b/include/editeng/editview.hxx @@ -333,8 +333,9 @@ public: const SvxFieldItem* GetFieldAtSelection(bool bAlsoCheckBeforeCursor = false) const; const SvxFieldItem* GetFieldAtSelection(bool* pIsBeforeCursor) const; - /// Select and return the field at the current cursor position - const SvxFieldData* GetFieldAtCursor() const; + /// return field under mouse, at selection, or immediately after (or before) the current cursor + const SvxFieldData* GetFieldAtCursor(bool bAlsoCheckBeforeCursor = false) const; + /// if no selection, select the field immediately after or before the current cursor void SelectFieldAtCursor(); /// Converts position in paragraph to logical position without unfolding fields sal_Int32 GetPosNoField(sal_Int32 nPara, sal_Int32 nPos) const; diff --git a/include/editeng/urlfieldhelper.hxx b/include/editeng/urlfieldhelper.hxx index 9a1d53d15512..0b9da3addf8c 100644 --- a/include/editeng/urlfieldhelper.hxx +++ b/include/editeng/urlfieldhelper.hxx @@ -18,10 +18,10 @@ class EDITENG_DLLPUBLIC URLFieldHelper { public: static void RemoveURLField(EditView& pEditView); - static bool IsCursorAtURLField(const EditView& pEditView); - static bool IsCursorAtURLField(const OutlinerView* pOLV) + static bool IsCursorAtURLField(const EditView& pEditView, bool bAlsoCheckBeforeCursor = false); + static bool IsCursorAtURLField(const OutlinerView* pOLV, bool bAlsoCheckBeforeCursor = false) { - return pOLV && IsCursorAtURLField(pOLV->GetEditView()); + return pOLV && IsCursorAtURLField(pOLV->GetEditView(), bAlsoCheckBeforeCursor); } }; diff --git a/sc/source/ui/drawfunc/drtxtob.cxx b/sc/source/ui/drawfunc/drtxtob.cxx index bc9a3b792487..ec527db045ac 100644 --- a/sc/source/ui/drawfunc/drtxtob.cxx +++ b/sc/source/ui/drawfunc/drtxtob.cxx @@ -407,8 +407,8 @@ void ScDrawTextObjectBar::GetState( SfxItemSet& rSet ) || rSet.GetItemState(SID_COPY_HYPERLINK_LOCATION) != SfxItemState::UNKNOWN || rSet.GetItemState(SID_REMOVE_HYPERLINK) != SfxItemState::UNKNOWN) { - SdrView* pView = mrViewData.GetScDrawView(); - if( !URLFieldHelper::IsCursorAtURLField(pView->GetTextEditOutlinerView()) ) + OutlinerView* pOutView = mrViewData.GetScDrawView()->GetTextEditOutlinerView(); + if (!URLFieldHelper::IsCursorAtURLField(pOutView, /*AlsoCheckBeforeCursor=*/true)) { rSet.DisableItem( SID_OPEN_HYPERLINK ); rSet.DisableItem( SID_EDIT_HYPERLINK ); diff --git a/sw/source/uibase/shells/drwtxtex.cxx b/sw/source/uibase/shells/drwtxtex.cxx index 565cb9f348dc..db76755474e4 100644 --- a/sw/source/uibase/shells/drwtxtex.cxx +++ b/sw/source/uibase/shells/drwtxtex.cxx @@ -938,7 +938,7 @@ void SwDrawTextShell::GetState(SfxItemSet& rSet) case SID_OPEN_HYPERLINK: case SID_COPY_HYPERLINK_LOCATION: { - if (!URLFieldHelper::IsCursorAtURLField(pOLV)) + if (!URLFieldHelper::IsCursorAtURLField(pOLV, /*AlsoCheckBeforeCursor=*/true)) rSet.DisableItem(nWhich); nSlotId = 0; } commit 213c6a9999e84f084978391c16276017078bf9ed Author: Justin Luth <[email protected]> AuthorDate: Thu Nov 2 13:35:24 2023 -0400 Commit: Justin Luth <[email protected]> CommitDate: Wed Nov 15 20:03:28 2023 +0100 related tdf#158031 editeng: GetFieldAtSel...(+look before cursor) Nearly a No-Functional-Change, but not quite. For practical and intentional purposes, it can be considered NFC. Although I didn't find an actual case where it happened, a code read says SelectFieldAtCursor COULD select a non-field. It assumed that previous code had already identified that there must be a field here. Well, I want to extend GetFieldAtSelection to check backwards to solve bug 158031 anyway, so I might as well "fix" this assumption to only select a valid field. This function REALLY depends on GetFieldAtSelection working properly. So, I put some asserts in to ensure that stays true. Change-Id: Ie22945a418497511501b04df5e17071d977cbd5b Reviewed-on: https://gerrit.libreoffice.org/c/core/+/158855 Tested-by: Jenkins Reviewed-by: Justin Luth <[email protected]> diff --git a/editeng/source/editeng/editview.cxx b/editeng/source/editeng/editview.cxx index d73cb24ef452..bfa98d1ae702 100644 --- a/editeng/source/editeng/editview.cxx +++ b/editeng/source/editeng/editview.cxx @@ -1344,7 +1344,16 @@ const SvxFieldItem* EditView::GetFieldUnderMousePointer( sal_Int32& nPara, sal_I return GetField( aPos, &nPara, &nPos ); } -const SvxFieldItem* EditView::GetFieldAtSelection() const +const SvxFieldItem* EditView::GetFieldAtSelection(bool bAlsoCheckBeforeCursor) const +{ + bool* pIsBeforeCursor = bAlsoCheckBeforeCursor ? &bAlsoCheckBeforeCursor : nullptr; + return GetFieldAtSelection(pIsBeforeCursor); +} + +// If pIsBeforeCursor != nullptr, the position before the cursor will also be checked for a field +// and pIsBeforeCursor will return true if that fallback field is returned. +// If no field is returned, the value in pIsBeforeCursor is meaningless. +const SvxFieldItem* EditView::GetFieldAtSelection(bool* pIsBeforeCursor) const { // a field is a dummy character - so it cannot span nodes or be a selection larger than 1 EditSelection aSel( pImpEditView->GetEditSelection() ); @@ -1361,6 +1370,13 @@ const SvxFieldItem* EditView::GetFieldAtSelection() const // Only when cursor is in font of field, no selection, // or only selecting field + bool bAlsoCheckBeforeCursor = false; + if (pIsBeforeCursor) + { + *pIsBeforeCursor = false; + bAlsoCheckBeforeCursor = nMaxIndex == nMinIndex; + } + const SvxFieldItem* pFoundBeforeCursor = nullptr; const CharAttribList::AttribsType& rAttrs = aSel.Min().GetNode()->GetCharAttribs().GetAttribs(); for (const auto& rAttr: rAttrs) { @@ -1369,34 +1385,43 @@ const SvxFieldItem* EditView::GetFieldAtSelection() const DBG_ASSERT(dynamic_cast<const SvxFieldItem*>(rAttr->GetItem()), "No FieldItem..."); if (rAttr->GetStart() == nMinIndex) return static_cast<const SvxFieldItem*>(rAttr->GetItem()); + + // perhaps the cursor is behind the field? + if (nMinIndex && rAttr->GetStart() == nMinIndex - 1) + pFoundBeforeCursor = static_cast<const SvxFieldItem*>(rAttr->GetItem()); } } + if (bAlsoCheckBeforeCursor) + { + *pIsBeforeCursor = /*(bool)*/pFoundBeforeCursor; + return pFoundBeforeCursor; + } return nullptr; } void EditView::SelectFieldAtCursor() { - const SvxFieldItem* pFieldItem = GetFieldAtSelection(); - if (pFieldItem) - { - // Make sure the whole field is selected - ESelection aSel = GetSelection(); - if (aSel.nStartPos == aSel.nEndPos) - { - aSel.nEndPos++; - SetSelection(aSel); - } - } + bool bIsBeforeCursor = false; + const SvxFieldItem* pFieldItem = GetFieldAtSelection(&bIsBeforeCursor); if (!pFieldItem) + return; + + // Make sure the whole field is selected + // A field is represented by a dummy character - so it cannot be a selection larger than 1 + ESelection aSel = GetSelection(); + if (aSel.nStartPos == aSel.nEndPos) // not yet selected { - // Cursor probably behind the field - extend selection to select the field - ESelection aSel = GetSelection(); - if (aSel.nStartPos > 0 && aSel.nStartPos == aSel.nEndPos) + if (bIsBeforeCursor) { - aSel.nStartPos--; - SetSelection(aSel); + assert (aSel.nStartPos); + --aSel.nStartPos; } + else + aSel.nEndPos++; + SetSelection(aSel); } + else + assert(std::abs(aSel.nStartPos - aSel.nEndPos) == 1); } const SvxFieldData* EditView::GetFieldAtCursor() const diff --git a/include/editeng/editview.hxx b/include/editeng/editview.hxx index 48019e3c4776..4c18ac2ba3e4 100644 --- a/include/editeng/editview.hxx +++ b/include/editeng/editview.hxx @@ -329,7 +329,10 @@ public: const SvxFieldItem* GetFieldUnderMousePointer( sal_Int32& nPara, sal_Int32& nPos ) const; const SvxFieldItem* GetField( const Point& rPos, sal_Int32* pnPara = nullptr, sal_Int32* pnPos = nullptr ) const; - const SvxFieldItem* GetFieldAtSelection() const; + /// return the selected field or the field immediately after (or before) the current cursor + const SvxFieldItem* GetFieldAtSelection(bool bAlsoCheckBeforeCursor = false) const; + const SvxFieldItem* GetFieldAtSelection(bool* pIsBeforeCursor) const; + /// Select and return the field at the current cursor position const SvxFieldData* GetFieldAtCursor() const; void SelectFieldAtCursor();
