[Libreoffice-commits] core.git: include/svx svx/source sw/qa

Libreoffice Gerrit user logerrit at kemper.freedesktop.org
Mon Oct 29 06:57:18 UTC 2018


 include/svx/srchdlg.hxx                 |    6 +++++-
 svx/source/dialog/srchdlg.cxx           |   19 ++++++++++++++-----
 sw/qa/uitest/findReplace/findReplace.py |    5 +++++
 3 files changed, 24 insertions(+), 6 deletions(-)

New commits:
commit ac51f1dfb3c63f1d0a3e2577ba5a56c25bc0b94b
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Sun Oct 28 23:28:10 2018 +0100
Commit:     Stephan Bergmann <sbergman at redhat.com>
CommitDate: Mon Oct 29 07:56:55 2018 +0100

    Avoid SvxSearchController::StateChanged during SvxSearchDialog sub-dialog
    
    UITest_findReplace occasionally fails (esp. in slowly-executing builds like the
    ASan+UBSan one, e.g., <https://ci.libreoffice.org/job/lo_ubsan/1084/>) because
    the "Find" edit is re-filled with old content while the "Format..." sub-dialog
    is executing:  The SfxBindings::NextJob timer fires from the main thread's
    Application::Yield, calls SvxSearchController::StateChanged ->
    SvxSearchDialog::SetItem_Impl -> SvxSearchDialog::Init_Impl, which goes into the
    
    >       else if (!aSearchStrings.empty())
    >       {
    >           bool bAttributes =
    >               ( ( pSearchList && pSearchList->Count() ) ||
    >                 ( pReplaceList && pReplaceList->Count() ) );
    >
    >           if ( bSetSearch && !bAttributes )
    >               m_pSearchLB->SetText(aSearchStrings[0]);
    
    code re-filling the "Find" edit (despite it having been cleared
    programatically), because bAttributes is false because the "Format..." sub-
    dialog has not yet completed, so pSearchList has not yet been filled (as is done
    by the handle_format_dlg code in test_find_writer in
    sw/qa/uitest/findReplace/findReplace.py).  (This issue can be triggered rather
    reliably by adding a sleep
    
    > @@ -94,6 +94,7 @@ class findReplace(UITestCase):
    >              xSizeFont.executeAction("BACKSPACE", tuple())
    >              xSizeFont.executeAction("TYPE", mkPropertyValues({"TEXT":"16"}))    #set font size 16
    >              xOkBtn = dialog.getChild("ok")
    > +            time.sleep(1)
    >              self.ui_test.close_dialog_through_button(xOkBtn)
    >
    >          self.ui_test.execute_blocking_action(format.executeAction, args=('CLICK', ()),
    
    to sw/qa/uitest/findReplace/findReplace.py.)
    
    So suppress executing SvxSearchController::StateChanged ->
    SvxSearchDialog::SetItem_Impl while an SvxSearchDialog sub-dialog is in
    progress.  The open TODO question is whether those state changes should be saved
    and executed once the sub-dialog has been executed, or whether it is OK to just
    throw them away (as happens now).
    
    Change-Id: I20fb8c8d88c3d3fe8b604718bb289a7421471aa7
    Reviewed-on: https://gerrit.libreoffice.org/62489
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/include/svx/srchdlg.hxx b/include/svx/srchdlg.hxx
index 440a824c8a55..4b82db88fe7c 100644
--- a/include/svx/srchdlg.hxx
+++ b/include/svx/srchdlg.hxx
@@ -38,11 +38,11 @@ class SvxSearchItem;
 class SfxStyleSheetBasePool;
 class SvxJSearchOptionsPage;
 class SvxSearchController;
+class VclAbstractDialog;
 struct SearchDlg_Impl;
 enum class ModifyFlags;
 enum class TransliterationFlags;
 
-
 struct SearchAttrItem
 {
     sal_uInt16          nSlot;
@@ -230,6 +230,8 @@ private:
     mutable TransliterationFlags
                             nTransliterationFlags;
 
+    bool m_executingSubDialog = false;
+
     DECL_LINK( ModifyHdl_Impl, Edit&, void );
     DECL_LINK( FlagHdl_Impl, Button*, void );
     DECL_LINK( CommandHdl_Impl, Button*, void );
@@ -263,6 +265,8 @@ private:
 
     void            ApplyTransliterationFlags_Impl( TransliterationFlags nSettings );
     bool            IsOtherOptionsExpanded();
+
+    short executeSubDialog(VclAbstractDialog * dialog);
 };
 
 #endif
diff --git a/svx/source/dialog/srchdlg.cxx b/svx/source/dialog/srchdlg.cxx
index 18b0ebd47d1f..795df4e6c1b9 100644
--- a/svx/source/dialog/srchdlg.cxx
+++ b/svx/source/dialog/srchdlg.cxx
@@ -44,6 +44,7 @@
 #include <com/sun/star/frame/ModuleManager.hpp>
 #include <com/sun/star/ui/XUIElement.hpp>
 #include <comphelper/processfactory.hxx>
+#include <comphelper/scopeguard.hxx>
 #include <svl/itempool.hxx>
 #include <svl/intitem.hxx>
 
@@ -1423,7 +1424,7 @@ IMPL_LINK( SvxSearchDialog, CommandHdl_Impl, Button *, pBtn, void )
                                                                     pSearchItem->GetLEVOther(),
                                                                     pSearchItem->GetLEVShorter(),
                                                                     pSearchItem->GetLEVLonger() ));
-        if ( pDlg->Execute() == RET_OK )
+        if ( executeSubDialog(pDlg.get()) == RET_OK )
         {
             pSearchItem->SetLEVRelaxed( pDlg->IsRelaxed() );
             pSearchItem->SetLEVOther( pDlg->GetOther() );
@@ -1439,7 +1440,7 @@ IMPL_LINK( SvxSearchDialog, CommandHdl_Impl, Button *, pBtn, void )
         SvxAbstractDialogFactory* pFact = SvxAbstractDialogFactory::Create();
         ScopedVclPtr<AbstractSvxJSearchOptionsDialog> aDlg(pFact->CreateSvxJSearchOptionsDialog(GetFrameWeld(), aSet,
                 pSearchItem->GetTransliterationFlags() ));
-        int nRet = aDlg->Execute();
+        int nRet = executeSubDialog(aDlg.get());
         if (RET_OK == nRet) //! true only if FillItemSet of SvxJSearchOptionsPage returns true
         {
             TransliterationFlags nFlags = aDlg->GetTransliterationFlags();
@@ -1917,7 +1918,8 @@ void SvxSearchDialog::EnableControl_Impl( Control const * pCtrl )
 
 void SvxSearchDialog::SetItem_Impl( const SvxSearchItem* pItem )
 {
-    if ( pItem )
+    //TODO: save pItem and process later if m_executingSubDialog?
+    if ( pItem && !m_executingSubDialog )
     {
         pSearchItem.reset(static_cast<SvxSearchItem*>(pItem->Clone()));
         Init_Impl( pSearchItem->GetPattern() &&
@@ -2041,7 +2043,7 @@ IMPL_LINK_NOARG(SvxSearchDialog, FormatHdl_Impl, Button*, void)
     ScopedVclPtr<SfxAbstractTabDialog> pDlg(pFact->CreateTabItemDialog(GetFrameWeld(), aSet));
     pDlg->SetText( aTxt );
 
-    if ( pDlg->Execute() == RET_OK )
+    if ( executeSubDialog(pDlg.get()) == RET_OK )
     {
         DBG_ASSERT( pDlg->GetOutputItemSet(), "invalid Output-Set" );
         SfxItemSet aOutSet( *pDlg->GetOutputItemSet() );
@@ -2132,7 +2134,7 @@ IMPL_LINK_NOARG(SvxSearchDialog, AttributeHdl_Impl, Button*, void)
 
     SvxAbstractDialogFactory* pFact = SvxAbstractDialogFactory::Create();
     ScopedVclPtr<VclAbstractDialog> pDlg(pFact->CreateSvxSearchAttributeDialog( this, *pSearchList, pImpl->pRanges.get() ));
-    pDlg->Execute();
+    executeSubDialog(pDlg.get());
     PaintAttrText_Impl();
 }
 
@@ -2376,6 +2378,13 @@ css::uno::Reference< css::awt::XWindowPeer >
         return xPeer;
 }
 
+short SvxSearchDialog::executeSubDialog(VclAbstractDialog * dialog) {
+    assert(!m_executingSubDialog);
+    comphelper::ScopeGuard g([this] { m_executingSubDialog = false; });
+    m_executingSubDialog = true;
+    return dialog->Execute();
+}
+
 SFX_IMPL_CHILDWINDOW_WITHID(SvxSearchDialogWrapper, SID_SEARCH_DLG);
 
 
diff --git a/sw/qa/uitest/findReplace/findReplace.py b/sw/qa/uitest/findReplace/findReplace.py
index 87a1ae17f147..213e47d6c372 100644
--- a/sw/qa/uitest/findReplace/findReplace.py
+++ b/sw/qa/uitest/findReplace/findReplace.py
@@ -99,6 +99,11 @@ class findReplace(UITestCase):
         self.ui_test.execute_blocking_action(format.executeAction, args=('CLICK', ()),
                 dialog_handler=handle_format_dlg)
 
+        # Verify these didn't get set again through SvxSearchController::StateChanged, timer-
+        # triggered from SfxBindings::NextJob while executing the Format dialog above:
+        self.assertEqual(get_state_as_dict(searchterm)["Text"], "")
+        self.assertEqual(get_state_as_dict(replaceterm)["Text"], "")
+
         xsearch = xDialog.getChild("search")
         xsearch.executeAction("CLICK", tuple())
         #verify


More information about the Libreoffice-commits mailing list