desktop/qa/desktop_lib/test_desktop_lib.cxx | 25 +++++++++++++++---------- include/sfx2/lokhelper.hxx | 3 +-- sfx2/source/appl/app.cxx | 11 ++++++++++- sfx2/source/appl/appdata.cxx | 25 +++++++++++++++++++++++++ sfx2/source/inc/appdata.hxx | 3 +++ sfx2/source/view/lokhelper.cxx | 11 +++-------- 6 files changed, 57 insertions(+), 21 deletions(-)
New commits: commit bf48944304b1f92a2fb8a3f186963bf33dc4385a Author: Caolán McNamara <[email protected]> AuthorDate: Wed Jul 23 15:58:48 2025 +0100 Commit: Caolán McNamara <[email protected]> CommitDate: Fri Jul 25 13:43:14 2025 +0200 Keep ViewShells sorted in most recently used order SfxViewShell::GetCurrent() is the current shell. Tt would be convenient to have the list that GetFirst/GetNext work with, in MRU order to be consistent with that, so it can be used to get the most recently current viewshell that matches a specific criteria. Also keep ViewFrames in MRU order for consistency with MRU ViewShell ordering. Change-Id: I15126242a4895bb90d8bcf34d91e917a217f4204 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/188333 Tested-by: Jenkins Reviewed-by: Caolán McNamara <[email protected]> diff --git a/desktop/qa/desktop_lib/test_desktop_lib.cxx b/desktop/qa/desktop_lib/test_desktop_lib.cxx index 4ce673195ab3..7627c5d94480 100644 --- a/desktop/qa/desktop_lib/test_desktop_lib.cxx +++ b/desktop/qa/desktop_lib/test_desktop_lib.cxx @@ -552,8 +552,9 @@ void DesktopLOKTest::testCreateView() // Test getViewIds(). std::vector<int> aViewIds(2); CPPUNIT_ASSERT(pDocument->m_pDocumentClass->getViewIds(pDocument, aViewIds.data(), aViewIds.size())); - CPPUNIT_ASSERT_EQUAL(nId0, aViewIds[0]); - CPPUNIT_ASSERT_EQUAL(nId1, aViewIds[1]); + // The expectation is that the most recently used shell is at the start + CPPUNIT_ASSERT_EQUAL(nId1, aViewIds[0]); + CPPUNIT_ASSERT_EQUAL(nId0, aViewIds[1]); // Make sure the created view is the active one, then switch to the old // one. @@ -3640,8 +3641,9 @@ void DesktopLOKTest::testMultiDocuments() // Validate the views of document 1. std::vector<int> aViewIdsDoc1(2); CPPUNIT_ASSERT(pDocument1->m_pDocumentClass->getViewIds(pDocument1, aViewIdsDoc1.data(), aViewIdsDoc1.size())); - CPPUNIT_ASSERT_EQUAL(nDoc1View0, aViewIdsDoc1[0]); - CPPUNIT_ASSERT_EQUAL(nDoc1View1, aViewIdsDoc1[1]); + // The expectation is that the most recently used shell is at the start + CPPUNIT_ASSERT_EQUAL(nDoc1View1, aViewIdsDoc1[0]); + CPPUNIT_ASSERT_EQUAL(nDoc1View0, aViewIdsDoc1[1]); CPPUNIT_ASSERT_EQUAL(nDoc1View1, pDocument1->m_pDocumentClass->getView(pDocument1)); CPPUNIT_ASSERT_EQUAL(nDocId1, SfxLokHelper::getDocumentIdOfView(nDoc1View1)); @@ -3669,8 +3671,9 @@ void DesktopLOKTest::testMultiDocuments() // Validate the views of document 2. std::vector<int> aViewIdsDoc2(2); CPPUNIT_ASSERT(pDocument2->m_pDocumentClass->getViewIds(pDocument2, aViewIdsDoc2.data(), aViewIdsDoc2.size())); - CPPUNIT_ASSERT_EQUAL(nDoc2View0, aViewIdsDoc2[0]); - CPPUNIT_ASSERT_EQUAL(nDoc2View1, aViewIdsDoc2[1]); + // The expectation is that the most recently used shell is at the start + CPPUNIT_ASSERT_EQUAL(nDoc2View1, aViewIdsDoc2[0]); + CPPUNIT_ASSERT_EQUAL(nDoc2View0, aViewIdsDoc2[1]); CPPUNIT_ASSERT_EQUAL(nDoc2View1, pDocument2->m_pDocumentClass->getView(pDocument2)); CPPUNIT_ASSERT_EQUAL(nDocId2, SfxLokHelper::getDocumentIdOfView(nDoc2View1)); @@ -3684,8 +3687,9 @@ void DesktopLOKTest::testMultiDocuments() // The views of document1 should be unchanged. CPPUNIT_ASSERT(pDocument1->m_pDocumentClass->getViewIds(pDocument1, aViewIdsDoc1.data(), aViewIdsDoc1.size())); - CPPUNIT_ASSERT_EQUAL(nDoc1View0, aViewIdsDoc1[0]); - CPPUNIT_ASSERT_EQUAL(nDoc1View1, aViewIdsDoc1[1]); + // The expectation is that the most recently used shell is at the start + CPPUNIT_ASSERT_EQUAL(nDoc1View1, aViewIdsDoc1[0]); + CPPUNIT_ASSERT_EQUAL(nDoc1View0, aViewIdsDoc1[1]); // Switch views in the first doc. CPPUNIT_ASSERT_EQUAL(nDocId1, SfxLokHelper::getDocumentIdOfView(nDoc1View0)); pDocument1->m_pDocumentClass->setView(pDocument1, nDoc1View0); @@ -3696,8 +3700,9 @@ void DesktopLOKTest::testMultiDocuments() // The views of document2 should be unchanged. CPPUNIT_ASSERT(pDocument2->m_pDocumentClass->getViewIds(pDocument2, aViewIdsDoc2.data(), aViewIdsDoc2.size())); - CPPUNIT_ASSERT_EQUAL(nDoc2View0, aViewIdsDoc2[0]); - CPPUNIT_ASSERT_EQUAL(nDoc2View1, aViewIdsDoc2[1]); + // The expectation is that the most recently used shell is at the start + CPPUNIT_ASSERT_EQUAL(nDoc2View1, aViewIdsDoc2[0]); + CPPUNIT_ASSERT_EQUAL(nDoc2View0, aViewIdsDoc2[1]); // Switch views in the second doc. CPPUNIT_ASSERT_EQUAL(nDocId2, SfxLokHelper::getDocumentIdOfView(nDoc2View0)); pDocument2->m_pDocumentClass->setView(pDocument2, nDoc2View0); diff --git a/sfx2/source/appl/app.cxx b/sfx2/source/appl/app.cxx index 30ce6d0f6b6e..29a496ee172f 100644 --- a/sfx2/source/appl/app.cxx +++ b/sfx2/source/appl/app.cxx @@ -215,6 +215,15 @@ void SfxApplication::SetViewFrame_Impl( SfxViewFrame *pFrame ) if( pFrame ) { + // Keep SfxViewFrame::Current() consistent with SfxViewFrame::First + pImpl->MoveFrameToFirstFrame(*pFrame); + SfxViewShell* pViewShell = pFrame->GetViewShell(); + if (pViewShell) + { + // Keep SfxViewShell::Current() consistent with SfxViewShell::First + pImpl->MoveShellToFirstShell(*pViewShell); + } + pFrame->DoActivate( bTaskActivate ); if ( bTaskActivate && pFrame->GetObjectShell() ) { @@ -231,7 +240,7 @@ void SfxApplication::SetViewFrame_Impl( SfxViewFrame *pFrame ) pProgress->SetState( pProgress->GetState() ); } - if ( pImpl->pViewFrame->GetViewShell() ) + if (pViewShell) { SfxDispatcher* pDisp = pImpl->pViewFrame->GetDispatcher(); pDisp->Flush(); diff --git a/sfx2/source/appl/appdata.cxx b/sfx2/source/appl/appdata.cxx index 70be13869689..ab18e7f7eaae 100644 --- a/sfx2/source/appl/appdata.cxx +++ b/sfx2/source/appl/appdata.cxx @@ -125,4 +125,29 @@ void SfxAppData_Impl::OnApplicationBasicManagerCreated( BasicManager& _rBasicMan #endif } +void SfxAppData_Impl::MoveShellToFirstShell(SfxViewShell& rShell) +{ + // Move associated ViewShell to the top of the ViewShells stack + // so the ViewShells are in order of most recently active + // ViewFrames. SfxViewShell::Current() should be equal to + // SfxViewShell::GetFirst(false) if SfxViewShell::Current() is + // not-null. + const auto shellIt = std::find(maViewShells.begin(), maViewShells.end(), &rShell); + if (shellIt != maViewShells.end() && shellIt != maViewShells.begin()) + { + maViewShells.erase(shellIt); + maViewShells.insert(maViewShells.begin(), &rShell); + } +} + +void SfxAppData_Impl::MoveFrameToFirstFrame(SfxViewFrame& rFrame) +{ + const auto frameIt = std::find(maViewFrames.begin(), maViewFrames.end(), &rFrame); + if (frameIt != maViewFrames.end() && frameIt != maViewFrames.begin()) + { + maViewFrames.erase(frameIt); + maViewFrames.insert(maViewFrames.begin(), &rFrame); + } +} + /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sfx2/source/inc/appdata.hxx b/sfx2/source/inc/appdata.hxx index 3eb7c88997f3..76a3bc9fe5e9 100644 --- a/sfx2/source/inc/appdata.hxx +++ b/sfx2/source/inc/appdata.hxx @@ -123,6 +123,9 @@ public: BasicManager is created before the application's BasicManager exists. */ void OnApplicationBasicManagerCreated( BasicManager& _rManager ); + + void MoveFrameToFirstFrame(SfxViewFrame& rFrame); + void MoveShellToFirstShell(SfxViewShell& rShell); }; class SfxDdeTriggerTopic_Impl final : public DdeTopic diff --git a/sfx2/source/view/lokhelper.cxx b/sfx2/source/view/lokhelper.cxx index 5d9380fa5c50..8ba23ebfd091 100644 --- a/sfx2/source/view/lokhelper.cxx +++ b/sfx2/source/view/lokhelper.cxx @@ -1031,7 +1031,7 @@ void SfxLokHelper::notifyUpdatePerViewId(SfxViewShell const& rTargetShell, SfxVi return; // This getCurrentView() is dubious - SAL_WARN_IF(!pViewShell, "kit", "no explicit viewshell set"); + SAL_WARN_IF(!pViewShell, "lok", "no explicit viewshell set"); int viewId = pViewShell ? SfxLokHelper::getView(*pViewShell) : SfxLokHelper::getCurrentView(); int sourceViewId = SfxLokHelper::getView(rSourceShell); rTargetShell.libreOfficeKitViewUpdatedCallbackPerViewId(nType, viewId, sourceViewId); commit 2e9d1ab6e2947fb2a5bbdf6d437edd32edcfc1d8 Author: Caolán McNamara <[email protected]> AuthorDate: Wed Jul 23 20:03:33 2025 +0100 Commit: Caolán McNamara <[email protected]> CommitDate: Fri Jul 25 13:43:04 2025 +0200 drop getView variant that auto-guesses from current viewshell all except one remaining case have a SfxViewShell arg known not to be null. Expand out that case explicitly. Change-Id: I9b3bfdd4fd9c2de50f933a05dd4e6ff48b64ed88 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/188332 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 9c59f23fc2fb..44099b1858d7 100644 --- a/include/sfx2/lokhelper.hxx +++ b/include/sfx2/lokhelper.hxx @@ -105,9 +105,8 @@ public: static SfxViewShell* getViewOfId(int nId); /// Get view id of view shell static int getView(const SfxViewShell& rViewShell); - /// Get the currently active view. + /// Get the currently active view shell id static int getCurrentView(); - static int getView(const SfxViewShell* pViewShell = nullptr); /// Get the number of views of the current DocId. static std::size_t getViewsCount(int nDocId); /// Get viewIds of views of the current DocId. diff --git a/sfx2/source/view/lokhelper.cxx b/sfx2/source/view/lokhelper.cxx index 107594116595..5d9380fa5c50 100644 --- a/sfx2/source/view/lokhelper.cxx +++ b/sfx2/source/view/lokhelper.cxx @@ -239,13 +239,6 @@ int SfxLokHelper::getCurrentView() return SfxLokHelper::getView(*pViewShell); } -int SfxLokHelper::getView(const SfxViewShell* pViewShell) -{ - if (pViewShell) - return SfxLokHelper::getView(*pViewShell); - return SfxLokHelper::getCurrentView(); -} - std::size_t SfxLokHelper::getViewsCount(int nDocId) { assert(nDocId != -1 && "Cannot getViewsCount for invalid DocId -1"); @@ -1037,7 +1030,9 @@ void SfxLokHelper::notifyUpdatePerViewId(SfxViewShell const& rTargetShell, SfxVi if (DisableCallbacks::disabled()) return; - int viewId = SfxLokHelper::getView(pViewShell); + // This getCurrentView() is dubious + SAL_WARN_IF(!pViewShell, "kit", "no explicit viewshell set"); + int viewId = pViewShell ? SfxLokHelper::getView(*pViewShell) : SfxLokHelper::getCurrentView(); int sourceViewId = SfxLokHelper::getView(rSourceShell); rTargetShell.libreOfficeKitViewUpdatedCallbackPerViewId(nType, viewId, sourceViewId); }
