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 <sberg...@redhat.com> AuthorDate: Fri Jul 31 22:38:09 2020 +0200 Commit: Stephan Bergmann <sberg...@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 <sberg...@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(); _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits