include/sfx2/lokhelper.hxx | 2 ++ include/sfx2/request.hxx | 5 +++++ sc/source/core/data/global.cxx | 11 +---------- sd/source/ui/view/viewshel.cxx | 15 ++++++++++++++- sfx2/source/control/request.cxx | 11 +++++++++++ sfx2/source/notify/hintpost.cxx | 37 +++++++++++++++++++++++++++++++++++++ sfx2/source/view/lokhelper.cxx | 11 +++++++++++ 7 files changed, 81 insertions(+), 11 deletions(-)
New commits: commit 66d70f50621806fbc39e512e09e2187f8709cd2c Author: Miklos Vajna <[email protected]> AuthorDate: Wed Dec 20 16:12:12 2023 +0100 Commit: Caolán McNamara <[email protected]> CommitDate: Fri Jan 5 12:08:33 2024 +0100 cool#7865 sfx2 lok: fix bad view id on async command dispatch If you try hard enough, it's possible to request an "add conditional format" dialog in one view and have the dialog pop up in an other view. This is related to cool#7853 but it focuses on the wider problem. What happens is that sometimes we do want to execute an uno command in an async way, e.g. one dialog opening an other dialog in its response handler: making sure the dialog is not manually / synchronously spinning the main loop while it's running is wanted. Also fixing each & every async command dispatch manually would be a lot of work. Fix the problem by remembering the current view in SfxHintPoster::Post() as it posts events to the main loop and restoring that view if necessary in SfxHintPoster::DoEvent_Impl(). Other comments: - An alternative would be to just dispatch all these UNO commands synchronously, but see above, there are cases where we want to stay async, a nested main loop would be worse. - An even more general fix would be to handle all calls to Application::PostUserEvent(), but vcl doesn't know what is the current view and would have trouble switching to that view (only sfx2 and higher up knows that), so do this only at a SfxHintPoster::Post() level, this is already meant to fix all async uno commands. - There was a single case in sd/ where we tried to dispatch an async event while switching views, so switch to a sync command dispatch there to avoid the problem. CppunitTest_sd_tiledrendering would hang without this and would emit a warning in SfxHintPoster::Post(). - Restore the sc/ code to do its async dispatch as it used to, so now CppunitTest_sc_tiledrendering's testOpenURL is a test case that covers this change. (cherry picked from commit ee7ca8e4ea8ed93655f99e77a9e77032ac830c46) Conflicts: sfx2/source/view/lokhelper.cxx Change-Id: I765e618f55caad791aa1fe7569a32bcc31622525 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/161541 Tested-by: Jenkins CollaboraOffice <[email protected]> Tested-by: Caolán McNamara <[email protected]> Reviewed-by: Caolán McNamara <[email protected]> diff --git a/include/sfx2/lokhelper.hxx b/include/sfx2/lokhelper.hxx index e015cad58cda..e593002d0498 100644 --- a/include/sfx2/lokhelper.hxx +++ b/include/sfx2/lokhelper.hxx @@ -95,6 +95,8 @@ public: static void destroyView(int nId); /// Set a view shell as current one. static void setView(int nId); + /// Determines if a call to setView() is in progress or not. + static bool isSettingView(); /// Set the edit mode for a document with callbacks disabled. static void setEditMode(int nMode, vcl::ITiledRenderable* pDoc); /// Get view shell with id diff --git a/include/sfx2/request.hxx b/include/sfx2/request.hxx index 5a97cb9e61b8..c62c0eab4305 100644 --- a/include/sfx2/request.hxx +++ b/include/sfx2/request.hxx @@ -120,6 +120,11 @@ public: /** Return the window that should be used as the parent for any dialogs this request creates */ weld::Window* GetFrameWeld() const; + + void SetLokViewId(int nId); + + int GetLokViewId() const; + private: const SfxRequest& operator=(const SfxRequest &) = delete; }; diff --git a/sc/source/core/data/global.cxx b/sc/source/core/data/global.cxx index 7dbc458e2059..f8d268ae843e 100644 --- a/sc/source/core/data/global.cxx +++ b/sc/source/core/data/global.cxx @@ -870,17 +870,8 @@ void ScGlobal::OpenURL(const OUString& rURL, const OUString& rTarget, bool bIgno SfxBoolItem aBrowsing( SID_BROWSE, true ); // No SID_SILENT anymore - SfxCallMode nCall = SfxCallMode::RECORD; - if (comphelper::LibreOfficeKit::isActive()) - { - nCall |= SfxCallMode::SYNCHRON; - } - else - { - nCall |= SfxCallMode::ASYNCHRON; - } pViewFrm->GetDispatcher()->ExecuteList(SID_OPENDOC, - nCall, + SfxCallMode::ASYNCHRON | SfxCallMode::RECORD, { &aUrl, &aTarget, &aFrm, &aReferer, &aNewView, &aBrowsing }); } diff --git a/sd/source/ui/view/viewshel.cxx b/sd/source/ui/view/viewshel.cxx index e12f65236dd9..1e29860b1152 100644 --- a/sd/source/ui/view/viewshel.cxx +++ b/sd/source/ui/view/viewshel.cxx @@ -305,10 +305,23 @@ void ViewShell::Activate(bool bIsMDIActivate) // thus, the Navigator will also get a current status SfxBoolItem aItem( SID_NAVIGATOR_INIT, true ); if (GetDispatcher() != nullptr) + { + SfxCallMode nCall = SfxCallMode::RECORD; + if (comphelper::LibreOfficeKit::isActive()) + { + // Make sure the LOK case doesn't dispatch async events while switching views, that + // would lead to a loop, see SfxHintPoster::DoEvent_Impl(). + nCall |= SfxCallMode::SYNCHRON; + } + else + { + nCall |= SfxCallMode::ASYNCHRON; + } GetDispatcher()->ExecuteList( SID_NAVIGATOR_INIT, - SfxCallMode::ASYNCHRON | SfxCallMode::RECORD, + nCall, { &aItem }); + } SfxViewShell* pViewShell = GetViewShell(); OSL_ASSERT (pViewShell!=nullptr); diff --git a/sfx2/source/control/request.cxx b/sfx2/source/control/request.cxx index b43d1dd991bc..77a597627689 100644 --- a/sfx2/source/control/request.cxx +++ b/sfx2/source/control/request.cxx @@ -71,6 +71,7 @@ struct SfxRequest_Impl: public SfxListener std::unique_ptr<SfxAllItemSet> pInternalArgs; SfxViewFrame* pViewFrame; + int m_nLokViewId = -1; css::uno::Reference< css::frame::XDispatchRecorder > xRecorder; css::uno::Reference< css::util::XURLTransformer > xTransform; @@ -760,4 +761,14 @@ void SfxRequest::ReleaseArgs() pImpl->pInternalArgs.reset(); } +void SfxRequest::SetLokViewId(int nId) +{ + pImpl->m_nLokViewId = nId; +} + +int SfxRequest::GetLokViewId() const +{ + return pImpl->m_nLokViewId; +} + /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sfx2/source/notify/hintpost.cxx b/sfx2/source/notify/hintpost.cxx index aa125d212943..78c8c0499099 100644 --- a/sfx2/source/notify/hintpost.cxx +++ b/sfx2/source/notify/hintpost.cxx @@ -20,8 +20,10 @@ #include <hintpost.hxx> #include <sfx2/request.hxx> +#include <sfx2/lokhelper.hxx> #include <utility> #include <vcl/svapp.hxx> +#include <comphelper/lok.hxx> SfxHintPoster::SfxHintPoster(std::function<void(std::unique_ptr<SfxRequest>)> aLink) : m_Link(std::move(aLink)) @@ -32,6 +34,19 @@ SfxHintPoster::~SfxHintPoster() {} void SfxHintPoster::Post(std::unique_ptr<SfxRequest> pHintToPost) { + if (comphelper::LibreOfficeKit::isActive()) + { + // Store the LOK view at the time of posting, so we can restore it later. + if (SfxLokHelper::isSettingView()) + { + // This would be bad, setView() should not trigger new posted hints, otherwise this will + // never end if the view doesn't match when we execute the posted hint. + SAL_WARN("sfx.notify", "SfxHintPoster::Post: posting new hint during setting a view"); + } + + pHintToPost->SetLokViewId(SfxLokHelper::getView()); + } + Application::PostUserEvent((LINK(this, SfxHintPoster, DoEvent_Impl)), pHintToPost.release()); AddFirstRef(); } @@ -40,7 +55,29 @@ IMPL_LINK(SfxHintPoster, DoEvent_Impl, void*, pPostedHint, void) { auto pRequest = static_cast<SfxRequest*>(pPostedHint); if (m_Link) + { + bool bSetView = false; + int nOldId = -1; + if (comphelper::LibreOfficeKit::isActive()) + { + int nNewId = pRequest->GetLokViewId(); + nOldId = SfxLokHelper::getView(); + if (nNewId != -1 && nNewId != nOldId) + { + // The current view ID is not the one that was active when posting the hint, switch + // to it. + SfxLokHelper::setView(nNewId); + bSetView = true; + } + } + m_Link(std::unique_ptr<SfxRequest>(pRequest)); + + if (bSetView) + { + SfxLokHelper::setView(nOldId); + } + } else delete pRequest; ReleaseRef(); diff --git a/sfx2/source/view/lokhelper.cxx b/sfx2/source/view/lokhelper.cxx index 20ef1528f03a..7a8c1c7454dc 100644 --- a/sfx2/source/view/lokhelper.cxx +++ b/sfx2/source/view/lokhelper.cxx @@ -35,12 +35,15 @@ #include <comphelper/lok.hxx> #include <sfx2/msgpool.hxx> #include <tools/json_writer.hxx> +#include <comphelper/scopeguard.hxx> #include <boost/property_tree/json_parser.hpp> using namespace com::sun::star; namespace { +bool g_bSettingView(false); + /// Used to disable callbacks. /// Needed to avoid recursion when switching views, /// which can cause clients to invoke LOKit API and @@ -165,8 +168,16 @@ void SfxLokHelper::destroyView(int nId) } } +bool SfxLokHelper::isSettingView() +{ + return g_bSettingView; +} + void SfxLokHelper::setView(int nId) { + g_bSettingView = true; + comphelper::ScopeGuard g([] { g_bSettingView = false; }); + SfxApplication* pApp = SfxApplication::Get(); if (pApp == nullptr) return;
