Reliably set up controls of hyperlink dialog in constructor

The recently added test_insert_hyperlink in
sw/qa/uitest/writer_tests3/hyperlinkdialog.py (UITest_writer_tests3) has often
failed on slow builds like <https://ci.libreoffice.org/job/lo_ubsan/>, by
hitting the assert in rtl_uString_newFromSubString as described at
<https://lists.freedesktop.org/archives/libreoffice/2020-July/085594.html> "Race
with SwEditWinUIObject::get_state during UITest_writer_tests3?"  However, it
turns out that the actual race is rather different from what was assumed there:

The initial content of the dialog's controls like "target" and "indication" were
only set during the first SvxHlinkCtrl::StateChanged(SID_HYERLINK_GETLINK),
which is called from the SfxBindings machinery based on a timer.  When that
happens, any text that has already been typed into those controls by the user
would be overwritten again.  But in normal GUI operations, the timer fires so
quickly that the user has not yet typed anything into those controls.  On the
other hand, for a typical (fast) execution of test_insert_hyperlink, the whole
test has already been executed when the timer fires, so the overwriting is not
noticed.

But for a slow execution of the test, the timer may e.g. fire after the
"indication" control's content ("link") has been typed in (which
SvxHlinkCtrl::StateChanged will reset to the empty string) and before the dialog
is closed (so instead of "link", the empty string will be added to the Writer
document, and obtaining the text selection of length 4 will crash as described
in the email).  (Also, the two calls to wait_until_property_is_updated added
with 27798238ecb200e0753b013c79df0e6c014c7a7a "uitest : Avoid any timing issue
in test_insert_hyperlink" and 1cdda798def040fe778348061c0e18b28aa0e6bd "Further
timing issues with test_insert_hyperlink" probably just address other symptoms
caused by the same underlying issue, and should no longer be necessary with this
fix.  But cleaning that up is left for a follow-up commit.)

The solution is to set up the controls' initial content already in the
constructor, so when the SfxBindings timer fires for the first time, it no
longer calls StateChanged because that state has already been recorded.
However, that caused the focus no longer to be set to the "target" control when
the dialog is opened, at least for the gen and svp VCL backends (which caused
the .uno:HyperlinkDialog-related tests in
desktop/qa/desktop_lib/test_desktop_lib.cxx, CppunitTest_desktop_lib, to fail
because GetFocusControl returned null):  The first call to
SvxHlinkCtrl::StateChanged -> SvxHplinkDlg::SetPage now happens during the
constructor, before the dialog is shown, so the request to grab the focus in
SetInitFocus was ignored.  The solution to that problem is to shift setting the
initial focus to the first call of SvxHpLInkDlg::Activate, which is called
whenever the dialog gains focus.

Change-Id: Ib4d5e06dfc21014ccec546565426fa2d27e63ce1
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/99903
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>

2 files changed, 10 insertions, 5 deletions

diff --git a/cui/source/dialogs/cuihyperdlg.cxx b/cui/source/dialogs/cuihyperdlg.cxx
index 9ca722680a15..dd184e16a732 100644
--- a/cui/source/dialogs/cuihyperdlg.cxx
+++ b/cui/source/dialogs/cuihyperdlg.cxx
@@ -140,6 +140,7 @@ SvxHpLinkDlg::SvxHpLinkDlg(SfxBindings* pBindings, SfxChildWindow* pChild, weld:
     // Init Dialog
     Start();
 
+    GetBindings().Update(SID_HYPERLINK_GETLINK);
     GetBindings().Update(SID_READONLY_MODE);
 
     m_xResetBtn->connect_clicked( LINK( this, SvxHpLinkDlg, ResetHdl ) );
@@ -163,6 +164,14 @@ SvxHpLinkDlg::~SvxHpLinkDlg()
     pOutSet.reset();
 }
 
+void SvxHpLinkDlg::Activate() {
+    if (mbGrabFocus) {
+        static_cast<SvxHyperlinkTabPageBase *>(GetTabPage(GetCurPageId()))->SetInitFocus();
+        mbGrabFocus = false;
+    }
+    SfxModelessDialogController::Activate();
+}
+
 void SvxHpLinkDlg::Close()
 {
     if (IsClosing())
@@ -259,11 +268,6 @@ void SvxHpLinkDlg::SetPage ( SvxHyperlinkItem const * pItem )
         aPageSet.Put ( *pItem );
 
         pCurrentPage->Reset( aPageSet );
-        if ( mbGrabFocus )
-        {
-            pCurrentPage->SetInitFocus();   // #92535# grab the focus only once at initialization
-            mbGrabFocus = false;
-        }
     }
 }
 
diff --git a/cui/source/inc/cuihyperdlg.hxx b/cui/source/inc/cuihyperdlg.hxx
index 702cf6396304..70d28c820f1d 100644
--- a/cui/source/inc/cuihyperdlg.hxx
+++ b/cui/source/inc/cuihyperdlg.hxx
@@ -104,6 +104,7 @@ private:
     void                    DeActivatePageImpl ();
     void                    ResetPageImpl ();
 
+    void Activate() override;
     virtual void Close() override;
     void Apply();