include/sfx2/bindings.hxx | 1 sc/qa/unit/tiledrendering/tiledrendering.cxx | 58 +++++++++++++++++++++++++++ sfx2/source/control/bindings.cxx | 30 +++++++++++++ sfx2/source/control/unoctitm.cxx | 21 +++++++++ 4 files changed, 109 insertions(+), 1 deletion(-)
New commits: commit 38b16540369287f795285adb987f6d7f789e4d57 Author: Miklos Vajna <[email protected]> AuthorDate: Mon Jan 8 16:25:44 2024 +0100 Commit: Caolán McNamara <[email protected]> CommitDate: Tue Jan 9 15:10:31 2024 +0100 cool#7492 sfx2 lok: just set language/locale on async binding update Trying to run online.git tests against commit 51d8a2ef54751403fa707816e27ddb4e7faa8231 (cool#7492 sfx2 lok: fix bad view id / statusbar string on async binding update, 2024-01-08), unit-tiletest failed. Manually trying to replace what the testSimultaneousTilesRenderedJustOnce test in that suite tries to do, it turned out that it can happen that the bindings are updated when a Writer view gets its focus, which is a problem, because the above commit started to switch views on binding updates, leading to a loop (bindings are updated about every second). Fix the problem by doing less work in SfxBindings::NextJob(): it's enough if the timer just switches languages and not views. That still keeps the Calc statusbar problem fixed from the above commit, but avoids the problematic view switching that broke unit-tiletest. (cherry picked from commit fb7b0b944741e4efae8d92a6e305036aff906c7a) Change-Id: I22cd9b3276c3f9987a2d90ad93f6026fdf6b8448 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/161819 Tested-by: Jenkins CollaboraOffice <[email protected]> Reviewed-by: Caolán McNamara <[email protected]> diff --git a/sfx2/source/control/bindings.cxx b/sfx2/source/control/bindings.cxx index 7dd4572cf5c0..81750cb1fe4b 100644 --- a/sfx2/source/control/bindings.cxx +++ b/sfx2/source/control/bindings.cxx @@ -1220,26 +1220,28 @@ void SfxBindings::UpdateControllers_Impl IMPL_LINK( SfxBindings, NextJob, Timer *, pTimer, void ) { bool bSetView = false; - int nOldId = -1; + SfxViewShell* pOldShell = nullptr; if (comphelper::LibreOfficeKit::isActive() && pDispatcher) { SfxViewFrame* pFrame = pDispatcher->GetFrame(); - SfxViewShell* pShell = pFrame ? pFrame->GetViewShell() : nullptr; - int nNewId = SfxLokHelper::getView(pShell); - nOldId = SfxLokHelper::getView(); - if (nNewId != -1 && nNewId != nOldId) + SfxViewShell* pNewShell = pFrame ? pFrame->GetViewShell() : nullptr; + pOldShell = SfxViewShell::Current(); + if (pNewShell && pNewShell != pOldShell) { - // The current view ID is not the one that belongs to this frame, switch to it. - SfxLokHelper::setView(nNewId); + // The current view ID is not the one that belongs to this frame, update + // language/locale. + comphelper::LibreOfficeKit::setLanguageTag(pNewShell->GetLOKLanguageTag()); + comphelper::LibreOfficeKit::setLocale(pNewShell->GetLOKLocale()); bSetView = true; } } NextJob_Impl(pTimer); - if (bSetView) + if (bSetView && pOldShell) { - SfxLokHelper::setView(nOldId); + comphelper::LibreOfficeKit::setLanguageTag(pOldShell->GetLOKLanguageTag()); + comphelper::LibreOfficeKit::setLocale(pOldShell->GetLOKLocale()); } } commit 7eaee503777f2c137a54db215db394cbf5e96ee3 Author: Miklos Vajna <[email protected]> AuthorDate: Fri Jan 5 16:51:42 2024 +0100 Commit: Caolán McNamara <[email protected]> CommitDate: Tue Jan 9 15:10:25 2024 +0100 cool#7492 sfx2 lok: fix bad view id / statusbar string on async binding update With two Calc views (first is an English one, second is a German one), type in the English view and in parallel to that, create a cell selection in the German view. Sometimes the status bar update about the selected cells arrives in English, in the German view. The root of the problem is that SfxBindings has a timer that does the expensive update of all the UNO commands only after a delay, but by the time this timer runs, possibly the current view is no longer the German one, which leads to English strings in the German view. Fix the problem somewhat similar to what commit ee7ca8e4ea8ed93655f99e77a9e77032ac830c46 (cool#7865 sfx2 lok: fix bad view id on async command dispatch, 2023-12-20) did in SfxHintPoster::DoEvent_Impl() by restoring the original view for the duration of the timer job execution. The test works even in case '--with-lang=en-US de' is not used, because it asserts the locale, which is not downgraded to English when translations are not available. This required turning the payload of the .uno:RowColSelCount status update into JSON, which is meant to be mostly backwards-compatible, given that we already have a mix of plain text and JSON for the various UNO commands. Another trouble is that the SfxBindings lazy update is a timer, so processing idles won't help and sleeping in test code is not ideal, either. Solve that by exposing the timer of SfxBindings, so test code can explicitly invoke the timer without waiting. (cherry picked from commit 51d8a2ef54751403fa707816e27ddb4e7faa8231) Conflicts: sc/qa/unit/tiledrendering/tiledrendering.cxx Change-Id: Iacf17f81c28b95ce41a0ee29ad25eb576db0d62a Reviewed-on: https://gerrit.libreoffice.org/c/core/+/161818 Tested-by: Jenkins CollaboraOffice <[email protected]> Reviewed-by: Caolán McNamara <[email protected]> diff --git a/include/sfx2/bindings.hxx b/include/sfx2/bindings.hxx index d058c939e4f9..3aea026374e2 100644 --- a/include/sfx2/bindings.hxx +++ b/include/sfx2/bindings.hxx @@ -186,6 +186,7 @@ public: SAL_DLLPRIVATE void SetRecorder_Impl( css::uno::Reference< css::frame::XDispatchRecorder > const & ); SAL_DLLPRIVATE void InvalidateSlotsInMap_Impl(); SAL_DLLPRIVATE void AddSlotToInvalidateSlotsMap_Impl( sal_uInt16 nId ); + Timer& GetTimer(); }; #ifdef DBG_UTIL diff --git a/sc/qa/unit/tiledrendering/tiledrendering.cxx b/sc/qa/unit/tiledrendering/tiledrendering.cxx index 4933adc299c5..434a6913d264 100644 --- a/sc/qa/unit/tiledrendering/tiledrendering.cxx +++ b/sc/qa/unit/tiledrendering/tiledrendering.cxx @@ -17,6 +17,7 @@ #include <com/sun/star/frame/DispatchHelper.hpp> #include <com/sun/star/datatransfer/clipboard/LokClipboard.hpp> #include <com/sun/star/datatransfer/UnsupportedFlavorException.hpp> +#include <com/sun/star/util/URLTransformer.hpp> #include <comphelper/processfactory.hxx> #include <comphelper/propertysequence.hxx> #include <osl/conditn.hxx> @@ -26,6 +27,8 @@ #include <comphelper/lok.hxx> #include <comphelper/propertyvalue.hxx> +#include <comphelper/dispatchcommand.hxx> +#include <sfx2/msgpool.hxx> #include <sfx2/childwin.hxx> #include <sfx2/lokhelper.hxx> #include <svx/svdpage.hxx> @@ -158,6 +161,7 @@ public: void testInvalidateOnTextEditWithDifferentZoomLevels(const ColRowZoom& rData); void testOpenURL(); void testInvalidateForSplitPanes(); + void testStatusBarLocale(); CPPUNIT_TEST_SUITE(ScTiledRenderingTest); CPPUNIT_TEST(testRowColumnHeaders); @@ -229,6 +233,7 @@ public: }); CPPUNIT_TEST(testOpenURL); CPPUNIT_TEST(testInvalidateForSplitPanes); + CPPUNIT_TEST(testStatusBarLocale); CPPUNIT_TEST_SUITE_END(); private: @@ -624,6 +629,7 @@ public: OString m_sInvalidateSheetGeometry; OString m_aHyperlinkClicked; OString m_ShapeSelection; + std::map<std::string, boost::property_tree::ptree> m_aStateChanges; TestLokCallbackWrapper m_callbackWrapper; ViewCallback(bool bDeleteListenerOnDestruct=true) @@ -768,6 +774,16 @@ public: { m_aTextSelectionResult.parseMessage(pPayload); } + break; + case LOK_CALLBACK_STATE_CHANGED: + { + std::stringstream aStream(pPayload); + boost::property_tree::ptree aTree; + boost::property_tree::read_json(aStream, aTree); + std::string aCommandName = aTree.get<std::string>("commandName"); + m_aStateChanges[aCommandName] = aTree; + } + break; } } }; @@ -3476,6 +3492,48 @@ void ScTiledRenderingTest::testInvalidateForSplitPanes() CPPUNIT_ASSERT_MESSAGE("The cell visible in the bottom left pane should be redrawn", bFoundBottomLeftPane); } +void ScTiledRenderingTest::testStatusBarLocale() +{ + // Given 2 views, the second's locale is set to German: + createDoc("empty.ods"); + int nView1 = SfxLokHelper::getView(); + ViewCallback aView1; + SfxLokHelper::createView(); + ViewCallback aView2; + SfxViewShell* pView2 = SfxViewShell::Current(); + pView2->SetLOKLocale("de-DE"); + { + SfxViewFrame* pFrame = pView2->GetViewFrame(); + SfxSlotPool& rSlotPool = SfxSlotPool::GetSlotPool(pFrame); + uno::Reference<util::XURLTransformer> xParser(util::URLTransformer::create(m_xContext)); + util::URL aCommandURL; + aCommandURL.Complete = ".uno:RowColSelCount"; + xParser->parseStrict(aCommandURL); + const SfxSlot* pSlot = rSlotPool.GetUnoSlot(aCommandURL.Path); + pFrame->GetBindings().GetDispatch(pSlot, aCommandURL, false); + } + aView2.m_aStateChanges.clear(); + + // When creating a cell selection in the 2nd view and processing jobs with the 1st view set to + // active: + comphelper::dispatchCommand(".uno:GoDownSel", {}); + SfxLokHelper::setView(nView1); + pView2->GetViewFrame()->GetBindings().GetTimer().Invoke(); + // Once more to hit the pImpl->bMsgDirty = false case in SfxBindings::NextJob_Impl(). + pView2->GetViewFrame()->GetBindings().GetTimer().Invoke(); + + // Then make sure that the locale is taken into account while producing the state changed + // callback: + auto it = aView2.m_aStateChanges.find(".uno:RowColSelCount"); + CPPUNIT_ASSERT(it != aView2.m_aStateChanges.end()); + std::string aLocale = it->second.get<std::string>("locale"); + // Without the accompanying fix in place, this test would have failed with: + // - Expected: de-DE + // - Actual : en-US + // i.e. the 2nd view got its callback with the locale of the first view, which is buggy. + CPPUNIT_ASSERT_EQUAL(std::string("de-DE"), aLocale); +} + } CPPUNIT_TEST_SUITE_REGISTRATION(ScTiledRenderingTest); diff --git a/sfx2/source/control/bindings.cxx b/sfx2/source/control/bindings.cxx index 4ea06231562a..7dd4572cf5c0 100644 --- a/sfx2/source/control/bindings.cxx +++ b/sfx2/source/control/bindings.cxx @@ -50,6 +50,8 @@ #include <sfx2/viewfrm.hxx> #include <sfx2/objsh.hxx> #include <sfx2/msgpool.hxx> +#include <sfx2/lokhelper.hxx> +#include <comphelper/lok.hxx> #include <cstddef> #include <memory> @@ -1217,7 +1219,28 @@ void SfxBindings::UpdateControllers_Impl IMPL_LINK( SfxBindings, NextJob, Timer *, pTimer, void ) { + bool bSetView = false; + int nOldId = -1; + if (comphelper::LibreOfficeKit::isActive() && pDispatcher) + { + SfxViewFrame* pFrame = pDispatcher->GetFrame(); + SfxViewShell* pShell = pFrame ? pFrame->GetViewShell() : nullptr; + int nNewId = SfxLokHelper::getView(pShell); + nOldId = SfxLokHelper::getView(); + if (nNewId != -1 && nNewId != nOldId) + { + // The current view ID is not the one that belongs to this frame, switch to it. + SfxLokHelper::setView(nNewId); + bSetView = true; + } + } + NextJob_Impl(pTimer); + + if (bSetView) + { + SfxLokHelper::setView(nOldId); + } } bool SfxBindings::NextJob_Impl(Timer const * pTimer) @@ -1780,4 +1803,9 @@ uno::Reference < frame::XDispatch > SfxBindings::GetDispatch( const SfxSlot* pSl return xRet; } +Timer& SfxBindings::GetTimer() +{ + return pImpl->aAutoTimer; +} + /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sfx2/source/control/unoctitm.cxx b/sfx2/source/control/unoctitm.cxx index 73d2d88ddc1b..38db25888c1f 100644 --- a/sfx2/source/control/unoctitm.cxx +++ b/sfx2/source/control/unoctitm.cxx @@ -1167,7 +1167,6 @@ static void InterceptLOKStateChangeEvent(sal_uInt16 nSID, SfxViewFrame* pViewFra } } else if (aEvent.FeatureURL.Path == "StatusDocPos" || - aEvent.FeatureURL.Path == "RowColSelCount" || aEvent.FeatureURL.Path == "StatusPageStyle" || aEvent.FeatureURL.Path == "StateWordCount" || aEvent.FeatureURL.Path == "PageStyleName" || @@ -1183,6 +1182,26 @@ static void InterceptLOKStateChangeEvent(sal_uInt16 nSID, SfxViewFrame* pViewFra aBuffer.append(aString); } } + else if (aEvent.FeatureURL.Path == "RowColSelCount") + { + OUString aString; + if (aEvent.IsEnabled) + { + aEvent.State >>= aString; + } + boost::property_tree::ptree aTree; + aTree.put("commandName", aEvent.FeatureURL.Complete); + aTree.put("locale", comphelper::LibreOfficeKit::getLocale().getBcp47()); + aTree.put("state", aString); + std::stringstream aStream; + boost::property_tree::write_json(aStream, aTree); + const SfxViewShell* pShell = pViewFrame->GetViewShell(); + if (pShell) + { + pShell->libreOfficeKitViewCallback(LOK_CALLBACK_STATE_CHANGED, aStream.str().c_str()); + } + return; + } else if (aEvent.FeatureURL.Path == "StateTableCell") { if (aEvent.IsEnabled)
