[Libreoffice-commits] core.git: cui/source

Stephan Bergmann (via logerrit) logerrit at kemper.freedesktop.org
Sat Aug 1 06:21:45 UTC 2020


 cui/source/dialogs/cuihyperdlg.cxx |   14 +++++++++-----
 cui/source/inc/cuihyperdlg.hxx     |    1 +
 2 files changed, 10 insertions(+), 5 deletions(-)

New commits:
commit 63049e98a659290229d3356e76d49cea44575011
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Fri Jul 31 22:38:09 2020 +0200
Commit:     Stephan Bergmann <sbergman at redhat.com>
CommitDate: Sat Aug 1 08:21:02 2020 +0200

    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 at redhat.com>

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();
 


More information about the Libreoffice-commits mailing list