cui/source/dialogs/cuihyperdlg.cxx |   74 +++++++++++++++----------------------
 cui/source/dialogs/iconcdlg.cxx    |    7 ---
 cui/source/inc/cuihyperdlg.hxx     |    4 --
 3 files changed, 31 insertions(+), 54 deletions(-)

New commits:
commit 77fc1c554137d6438706743851ed04c702c1694a
Author:     Skyler Grey <[email protected]>
AuthorDate: Fri Sep 19 12:18:03 2025 +0000
Commit:     Szymon Kłos <[email protected]>
CommitDate: Tue Sep 23 11:14:16 2025 +0200

    fix(lok): avoid remembering hyperlink dialog tab
    
    In tdf#90496, insert hyperlink was set up to remember the last view used
    
    Normally, this is desireable. With LibreOfficeKit, however, it's not
    necessarily the case that the same user is accessing the dialog on
    consecutive openings
    
    This is a problem in Collabora Online, say, where the dialog would open
    on different tabs depending on where the last person closed it...
    
    ...all this to say: let's pick a reasonable default ("internet") for Kit
    and use that all the time
    
    Change-Id: I6a6a6964c6796141af4a0b73a208e49cb6e3fa4a
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/191204
    Reviewed-by: Szymon Kłos <[email protected]>
    Tested-by: Jenkins CollaboraOffice <[email protected]>
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/191325
    Tested-by: Skyler Grey <[email protected]>

diff --git a/cui/source/dialogs/cuihyperdlg.cxx 
b/cui/source/dialogs/cuihyperdlg.cxx
index 35cbf1112980..2587920d57f6 100644
--- a/cui/source/dialogs/cuihyperdlg.cxx
+++ b/cui/source/dialogs/cuihyperdlg.cxx
@@ -244,6 +244,10 @@ void SvxHpLinkDlg::SetPage ( SvxHyperlinkItem const * 
pItem )
 
     OUString sPageId(msRememberedPageId);
 
+    if (comphelper::LibreOfficeKit::isActive()) {
+        sPageId = "internet";
+    }
+
     if (eProtocolTyp == INetProtocol::Http || eProtocolTyp == 
INetProtocol::Https || eProtocolTyp == INetProtocol::Ftp) {
         sPageId = "internet";
     } else if (eProtocolTyp == INetProtocol::Mailto) {
commit cd22bddd6492cb80c84fab5d00d6569fb1527b96
Author:     Maya Stephens <[email protected]>
AuthorDate: Thu Aug 28 11:40:55 2025 +0000
Commit:     Szymon Kłos <[email protected]>
CommitDate: Tue Sep 23 11:14:08 2025 +0200

    Remove redundant code when hyperlink dialog is created
    
    The code for creating the hyperlink dialog end up calling the same functions
    multiple times, more than we need to. Previously the code would:
     - Create an empty SID_HYPERLINK_GETLINK item
     - Create the tabs, resetting the fields on each based on this empty item
     - Swap to the last used tab
     - Switch to the active page, and reset it again
     - Get the data about the object that was clicked
     - Reset all the tabs again from having loaded a new link
     - ... and reset them all
    
    A lot of these calls can be removed. Now, we:
     - Create the tabs, but without calling reset on them
            (because we don't have the information yet)
     - Get information about the currently selected shape
            (This automatically calls reset on them, and
            selects the most appropriate tab to show)
    
    Change-Id: I6a6a6964c16e8b3ba447029b4cfe0f41090ae2a8
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/190805
    Reviewed-by: Michael Stahl <[email protected]>
    Tested-by: Jenkins CollaboraOffice <[email protected]>
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/191327
    Tested-by: Skyler Grey <[email protected]>

diff --git a/cui/source/dialogs/cuihyperdlg.cxx 
b/cui/source/dialogs/cuihyperdlg.cxx
index 14d914cadd16..35cbf1112980 100644
--- a/cui/source/dialogs/cuihyperdlg.cxx
+++ b/cui/source/dialogs/cuihyperdlg.cxx
@@ -138,7 +138,6 @@ SvxHpLinkDlg::SvxHpLinkDlg(SfxBindings* pBindings, 
SfxChildWindow* pChild, weld:
 
     SetInputSet (mpItemSet.get());
 
-    // insert pages
     AddTabPage(u"internet"_ustr, SvxHyperlinkInternetTp::Create);
     AddTabPage(u"mail"_ustr, SvxHyperlinkMailTp::Create);
     if (!comphelper::LibreOfficeKit::isActive())
@@ -147,21 +146,9 @@ SvxHpLinkDlg::SvxHpLinkDlg(SfxBindings* pBindings, 
SfxChildWindow* pChild, weld:
         AddTabPage(u"newdocument"_ustr, SvxHyperlinkNewDocTp::Create);
     }
 
-    // tdf#90496 - remember last used view in hyperlink dialog
-    SetCurPageId(msRememberedPageId);
-
-    // Init Dialog
-    Start();
-
     GetBindings().Update(SID_HYPERLINK_GETLINK);
     GetBindings().Update(SID_READONLY_MODE);
 
-    // Now that we have got the link data, we can reset
-    // all the pages with the correct information
-    for (std::unique_ptr<IconChoicePageData>& page: maPageList) {
-        page->xPage->Reset(*pSet);
-    }
-
     m_xResetBtn->connect_clicked( LINK( this, SvxHpLinkDlg, ResetHdl ) );
     m_xOKBtn->connect_clicked( LINK ( this, SvxHpLinkDlg, ClickOkHdl_Impl ) );
     m_xApplyBtn->connect_clicked ( LINK ( this, SvxHpLinkDlg, 
ClickApplyHdl_Impl ) );
diff --git a/cui/source/dialogs/iconcdlg.cxx b/cui/source/dialogs/iconcdlg.cxx
index 9bda9d215dcf..373e83cc3999 100644
--- a/cui/source/dialogs/iconcdlg.cxx
+++ b/cui/source/dialogs/iconcdlg.cxx
@@ -75,7 +75,6 @@ void SvxHpLinkDlg::AddTabPage(const OUString& rId, CreatePage 
pCreateFunc /* !=
 {
     weld::Container* pPage = m_xIconCtrl->get_page(rId);
     maPageList.emplace_back(new IconChoicePageData(rId, pCreateFunc(pPage, 
this, pSet)));
-    maPageList.back()->xPage->Reset(*pSet);
     PageCreated(*maPageList.back()->xPage);
 }
 
@@ -273,12 +272,6 @@ bool SvxHpLinkDlg::QueryClose()
     return bRet;
 }
 
-void SvxHpLinkDlg::Start()
-{
-    SwitchPage(msCurrentPageId);
-    ActivatePageImpl();
-}
-
 /**********************************************************************
 |
 | tool-methods
diff --git a/cui/source/inc/cuihyperdlg.hxx b/cui/source/inc/cuihyperdlg.hxx
index 0f2e8f1ab4bd..f142d4cdfdf8 100644
--- a/cui/source/inc/cuihyperdlg.hxx
+++ b/cui/source/inc/cuihyperdlg.hxx
@@ -124,7 +124,6 @@ public:
     WhichRangesContainer GetInputRanges( const SfxItemPool& );
     void                SetInputSet( const SfxItemSet* pInSet );
 
-    void                Start();
     bool                QueryClose();
 
     void                PageCreated(IconChoicePage& rPage);
commit 566ad85f66ce7a3add5d644155eba0db99d154d8
Author:     Maya Stephens <[email protected]>
AuthorDate: Thu Aug 28 11:40:55 2025 +0000
Commit:     Szymon Kłos <[email protected]>
CommitDate: Tue Sep 23 11:14:02 2025 +0200

    Change how SvxHpLinkDlg::SetPage functions
    
    A few changes:
     - Use msRememberedPageId instead of GetCurrPageId as default, as this
    should be updated whenever the page is changed
     - Count Https links as using the internet tab
     - Reset every page instead of only the current page with the information
       - This prevents the dialog box from changing size in online
     - Prevent crashes from trying to open a type of page not supported in LoKit
       - Trying to open the document tab in LoKit causes a crash
       - Fix this by not allowing this option to be selected for links starting 
ftp://
       - Additionally, allow nullptr to be returned and handle by just exiting 
the
    function, in case another problem appears in the future
    
    Change-Id: I509dc699d3517600dfc329722bc5f1192b4e7956
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/190532
    Tested-by: Jenkins CollaboraOffice <[email protected]>
    Reviewed-by: Michael Stahl <[email protected]>
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/191326
    Tested-by: Skyler Grey <[email protected]>

diff --git a/cui/source/dialogs/cuihyperdlg.cxx 
b/cui/source/dialogs/cuihyperdlg.cxx
index e0f7f6acccdc..14d914cadd16 100644
--- a/cui/source/dialogs/cuihyperdlg.cxx
+++ b/cui/source/dialogs/cuihyperdlg.cxx
@@ -185,7 +185,14 @@ SvxHpLinkDlg::~SvxHpLinkDlg()
     pOutSet.reset();
 }
 
-void SvxHpLinkDlg::Activate() {
+IconChoicePage* SvxHpLinkDlg::GetTabPage( std::u16string_view rPageId )
+{
+    const IconChoicePageData* p = GetPageData(rPageId);
+    return (p == nullptr) ? nullptr : p->xPage.get();
+}
+
+void SvxHpLinkDlg::Activate()
+{
     if (mbGrabFocus) {
         static_cast<SvxHyperlinkTabPageBase 
*>(GetTabPage(GetCurPageId()))->SetInitFocus();
         mbGrabFocus = false;
@@ -242,48 +249,36 @@ IMPL_LINK_NOARG(SvxHpLinkDlg, ClickApplyHdl_Impl, 
weld::Button&, void)
 |************************************************************************/
 void SvxHpLinkDlg::SetPage ( SvxHyperlinkItem const * pItem )
 {
-    OUString sPageId(u"internet"_ustr);
+    mpItemSet->Put(*pItem);
 
     const OUString& aStrURL(pItem->GetURL());
     INetURLObject aURL(aStrURL);
     INetProtocol eProtocolTyp = aURL.GetProtocol();
 
-    switch ( eProtocolTyp )
-    {
-        case INetProtocol::Http :
-        case INetProtocol::Ftp :
-            sPageId = "internet";
-            break;
-        case INetProtocol::File :
-            sPageId = "document";
-            break;
-        case INetProtocol::Mailto :
-            sPageId = "mail";
-            break;
-        default :
-            if (aStrURL.startsWith("#"))
-                sPageId = "document";
-            else
-            {
-                // not valid
-                sPageId = GetCurPageId();
-            }
-            break;
+    OUString sPageId(msRememberedPageId);
+
+    if (eProtocolTyp == INetProtocol::Http || eProtocolTyp == 
INetProtocol::Https || eProtocolTyp == INetProtocol::Ftp) {
+        sPageId = "internet";
+    } else if (eProtocolTyp == INetProtocol::Mailto) {
+        sPageId = "mail";
+    } else if (!comphelper::LibreOfficeKit::isActive() &&
+        (eProtocolTyp == INetProtocol::File || aStrURL.startsWith("#"))) {
+        sPageId = "document";
     }
 
-    ShowPage (sPageId);
+    IconChoicePage* pPage = GetTabPage(sPageId);
 
-    SvxHyperlinkTabPageBase* pCurrentPage = 
static_cast<SvxHyperlinkTabPageBase*>(GetTabPage( sPageId ));
+    // Switching to tab that doesn't exist should not crash
+    if (pPage == nullptr) { return; }
 
+    ShowPage (sPageId);
     mbIsHTMLDoc = (pItem->GetInsertMode() & HLINK_HTMLMODE) != 0;
 
-    IconChoicePage* pPage = GetTabPage (sPageId);
-    if(pPage)
-    {
-        SfxItemSet& aPageSet = const_cast<SfxItemSet&>(pPage->GetItemSet ());
-        aPageSet.Put ( *pItem );
+    SfxItemSet& aPageSet = const_cast<SfxItemSet&>(pPage->GetItemSet ());
+    aPageSet.Put ( *pItem );
 
-        pCurrentPage->Reset( aPageSet );
+    for (std::unique_ptr<IconChoicePageData>& page: maPageList) {
+        page->xPage->Reset(*pSet);
     }
 }
 
diff --git a/cui/source/inc/cuihyperdlg.hxx b/cui/source/inc/cuihyperdlg.hxx
index 8423d8c49759..0f2e8f1ab4bd 100644
--- a/cui/source/inc/cuihyperdlg.hxx
+++ b/cui/source/inc/cuihyperdlg.hxx
@@ -99,8 +99,7 @@ private:
     DECL_LINK (ClickOkHdl_Impl, weld::Button&, void );
     DECL_LINK (ClickApplyHdl_Impl, weld::Button&, void );
 
-    IconChoicePage*         GetTabPage( std::u16string_view rPageId )
-                                { return GetPageData(rPageId)->xPage.get(); }
+    IconChoicePage* GetTabPage( std::u16string_view rPageId );
 
     void                    ActivatePageImpl ();
     void                    DeActivatePageImpl ();

Reply via email to