sc/qa/unit/tiledrendering/SheetViewTest.cxx | 72 ++++++++++++++++++++++++++++
sc/source/core/data/SheetViewManager.cxx | 1
sc/source/ui/inc/viewdata.hxx | 2
sc/source/ui/view/viewdata.cxx | 31 +++++++-----
sc/source/ui/view/viewfun3.cxx | 18 +++++--
5 files changed, 107 insertions(+), 17 deletions(-)
New commits:
commit e6cf2c90f14097a1c2ebbd013ccb06276054ad17
Author: Tomaž Vajngerl <tomaz.vajng...@collabora.co.uk>
AuthorDate: Mon Aug 11 17:27:18 2025 +0200
Commit: Tomaž Vajngerl <qui...@gmail.com>
CommitDate: Fri Sep 12 20:57:01 2025 +0200
sc: test ".uno:RemoveSheetView" to remove current sheet view
Also fix issues found with the test.
We need to broadcast that the table was inserted or it won't
properly be accounted for ViewData, which will cause an assert
to fail.
Change-Id: I0c086e16090d40e8a663217b36d04d4de99966cb
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/189386
Tested-by: Jenkins
Reviewed-by: Tomaž Vajngerl <qui...@gmail.com>
diff --git a/sc/qa/unit/tiledrendering/SheetViewTest.cxx
b/sc/qa/unit/tiledrendering/SheetViewTest.cxx
index 834bfe2d8ea1..b11d898c4882 100644
--- a/sc/qa/unit/tiledrendering/SheetViewTest.cxx
+++ b/sc/qa/unit/tiledrendering/SheetViewTest.cxx
@@ -186,6 +186,78 @@ CPPUNIT_TEST_FIXTURE(SheetViewTest,
testSyncValuesBetweenMainSheetAndSheetView)
CPPUNIT_ASSERT_EQUAL(u"ABC123"_ustr, pDocument->GetString(aA2SheetView));
}
+CPPUNIT_TEST_FIXTURE(SheetViewTest, testRemoveSheetView)
+{
+ // Create two views, and leave the second one current.
+ ScModelObj* pModelObj = createDoc("SheetView_AutoFilter.ods");
+
pModelObj->initializeForTiledRendering(uno::Sequence<beans::PropertyValue>());
+ ScDocument* pDocument = pModelObj->GetDocument();
+
+ // Setup views
+ ScTestViewCallback aView1;
+ ScTabViewShell* pTabView1 = aView1.getTabViewShell();
+ SfxLokHelper::createView();
+ Scheduler::ProcessEventsToIdle();
+ ScTestViewCallback aView2;
+ ScTabViewShell* pTabView2 = aView2.getTabViewShell();
+ CPPUNIT_ASSERT(pTabView1 != pTabView2);
+ CPPUNIT_ASSERT(aView1.getViewID() != aView2.getViewID());
+
+ // Switch to View1
+ SfxLokHelper::setView(aView1.getViewID());
+ Scheduler::ProcessEventsToIdle();
+
+ // Create a new sheet view for view 1
+ dispatchCommand(mxComponent, u".uno:NewSheetView"_ustr, {});
+ Scheduler::ProcessEventsToIdle();
+
+ // Check AutoFilter values for each view
+ CPPUNIT_ASSERT_EQUAL(u"4"_ustr, pTabView1->GetCurrentString(0, 1));
+ CPPUNIT_ASSERT_EQUAL(u"5"_ustr, pTabView1->GetCurrentString(0, 2));
+ CPPUNIT_ASSERT_EQUAL(u"3"_ustr, pTabView1->GetCurrentString(0, 3));
+ CPPUNIT_ASSERT_EQUAL(u"7"_ustr, pTabView1->GetCurrentString(0, 4));
+
+ CPPUNIT_ASSERT_EQUAL(u"4"_ustr, pTabView2->GetCurrentString(0, 1));
+ CPPUNIT_ASSERT_EQUAL(u"5"_ustr, pTabView2->GetCurrentString(0, 2));
+ CPPUNIT_ASSERT_EQUAL(u"3"_ustr, pTabView2->GetCurrentString(0, 3));
+ CPPUNIT_ASSERT_EQUAL(u"7"_ustr, pTabView2->GetCurrentString(0, 4));
+
+ // Switch to View2
+ SfxLokHelper::setView(aView2.getViewID());
+ Scheduler::ProcessEventsToIdle();
+
+ // Sort AutoFilter descending
+ dispatchCommand(mxComponent, u".uno:SortDescending"_ustr, {});
+ Scheduler::ProcessEventsToIdle();
+
+ // Check values are sorted for view 2
+ CPPUNIT_ASSERT_EQUAL(u"7"_ustr, pTabView2->GetCurrentString(0, 1));
+ CPPUNIT_ASSERT_EQUAL(u"5"_ustr, pTabView2->GetCurrentString(0, 2));
+ CPPUNIT_ASSERT_EQUAL(u"4"_ustr, pTabView2->GetCurrentString(0, 3));
+ CPPUNIT_ASSERT_EQUAL(u"3"_ustr, pTabView2->GetCurrentString(0, 4));
+
+ // Sheet view must be present
+ auto pSheetViewManager = pDocument->GetSheetViewManager(0);
+ CPPUNIT_ASSERT_EQUAL(size_t(1), pSheetViewManager->getSheetViews().size());
+
+ // There should be 2 tables
+ CPPUNIT_ASSERT_EQUAL(SCTAB(2), pDocument->GetTableCount());
+
+ // Switch to View1
+ SfxLokHelper::setView(aView1.getViewID());
+ Scheduler::ProcessEventsToIdle();
+
+ // We remove the current sheet view
+ dispatchCommand(mxComponent, u".uno:RemoveSheetView"_ustr, {});
+ Scheduler::ProcessEventsToIdle();
+
+ // No more sheet view
+ CPPUNIT_ASSERT_EQUAL(size_t(0), pSheetViewManager->getSheetViews().size());
+
+ // Only 1 table left
+ CPPUNIT_ASSERT_EQUAL(SCTAB(1), pDocument->GetTableCount());
+}
+
CPPUNIT_PLUGIN_IMPLEMENT();
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/source/core/data/SheetViewManager.cxx
b/sc/source/core/data/SheetViewManager.cxx
index e4e3858fbcdc..a57ea9666025 100644
--- a/sc/source/core/data/SheetViewManager.cxx
+++ b/sc/source/core/data/SheetViewManager.cxx
@@ -26,6 +26,7 @@ bool SheetViewManager::remove(SheetViewID nID)
if (nID >= 0 && o3tl::make_unsigned(nID) < maViews.size())
{
maViews.erase(maViews.begin() + nID);
+ return true;
}
return false;
}
diff --git a/sc/source/ui/view/viewfun3.cxx b/sc/source/ui/view/viewfun3.cxx
index 569b2d19f409..b47865ec5ec7 100644
--- a/sc/source/ui/view/viewfun3.cxx
+++ b/sc/source/ui/view/viewfun3.cxx
@@ -64,6 +64,7 @@
#include <clipoptions.hxx>
#include <gridwin.hxx>
#include <SheetView.hxx>
+#include <uiitems.hxx>
#include <com/sun/star/util/XCloneable.hpp>
using namespace com::sun::star;
@@ -2068,6 +2069,9 @@ void ScViewFunc::MakeNewSheetView()
SCTAB nSheetViewTab = nTab + 1;
if (rDocument.CopyTab(nTab, nSheetViewTab))
{
+ GetViewData().GetDocShell().Broadcast(ScTablesHint(SC_TAB_INSERTED,
nSheetViewTab));
+ SfxGetpApp()->Broadcast(SfxHint(SfxHintId::ScTablesChanged));
+
// Add and register the created sheet view
rDocument.SetSheetView(nSheetViewTab, true);
sc::SheetViewID nSheetViewID = rDocument.CreateNewSheetView(nTab,
nSheetViewTab);
@@ -2076,6 +2080,9 @@ void ScViewFunc::MakeNewSheetView()
// Update
GetViewData().SetTabNo(nSheetViewTab); // force add the sheet view tab
GetViewData().SetTabNo(nTab); // then change back to the current tab
+
+ ScDocShell& rDocSh = GetViewData().GetDocShell();
+ rDocSh.PostPaintGridAll();
PaintExtras(); // update Tab Control
}
}
@@ -2086,18 +2093,19 @@ void ScViewFunc::RemoveCurrentSheetView()
if (nSheetViewID == sc::DefaultSheetViewID)
return;
- SCTAB nTab = GetViewData().GetTabNumber();
ScDocument& rDocument = GetViewData().GetDocument();
- SCTAB nSheetViewTab = rDocument.GetSheetViewNumber(nTab, nSheetViewID);
-
+ SCTAB nTab = GetViewData().GetTabNumber();
auto pSheetManager = rDocument.GetSheetViewManager(nTab);
+ if (!pSheetManager)
+ return;
+
+ SCTAB nSheetViewTab = rDocument.GetSheetViewNumber(nTab, nSheetViewID);
pSheetManager->remove(nSheetViewID);
GetViewData().SetSheetViewID(sc::DefaultSheetViewID);
GetViewData().SetTabNo(nTab);
GetViewData().GetDocFunc().DeleteTable(nSheetViewTab, true);
- ScDocShell& rDocSh = GetViewData().GetDocShell();
- rDocSh.PostPaintGridAll();
+ GetViewData().GetDocShell().PostPaintGridAll();
PaintExtras();
}
commit 202a1fead00d235c2b7e330d906e852a0e597380
Author: Tomaž Vajngerl <tomaz.vajng...@collabora.co.uk>
AuthorDate: Thu Sep 4 17:10:53 2025 +0200
Commit: Tomaž Vajngerl <qui...@gmail.com>
CommitDate: Fri Sep 12 20:56:51 2025 +0200
sc: fix scrolling in sheet view, add check if a tab no. is valid
Scrolling needs to use the tab number of the actual sheet we are
currently using in the UI. The same goes for the grid.
Added a function to check if a tab number is currently valid, to
reduce duplication and to make the code a bit more readable.
Change-Id: I4e76a61efaa3632b020cfd75ea77c0809f08f6da
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/190598
Reviewed-by: Tomaž Vajngerl <qui...@gmail.com>
Tested-by: Jenkins
diff --git a/sc/source/ui/inc/viewdata.hxx b/sc/source/ui/inc/viewdata.hxx
index 2e0e3b8270c2..86021bf92bf0 100644
--- a/sc/source/ui/inc/viewdata.hxx
+++ b/sc/source/ui/inc/viewdata.hxx
@@ -349,6 +349,8 @@ private:
void UpdateCurrentTab();
ScViewDataTable* FetchTableData(SCTAB) const;
+ bool IsValidTabNumber(SCTAB nTabNumber) const;
+
public:
ScViewData( ScDocShell& rDocSh, ScTabViewShell* pViewSh );
~ScViewData() COVERITY_NOEXCEPT_FALSE;
diff --git a/sc/source/ui/view/viewdata.cxx b/sc/source/ui/view/viewdata.cxx
index 577b0a085f7d..4ff40391ca6e 100644
--- a/sc/source/ui/view/viewdata.cxx
+++ b/sc/source/ui/view/viewdata.cxx
@@ -843,9 +843,15 @@ ScViewData::~ScViewData() COVERITY_NOEXCEPT_FALSE
ScDBFunc* ScViewData::GetView() const { return pView; }
+bool ScViewData::IsValidTabNumber(SCTAB nTabNumber) const
+{
+ return nTabNumber >= 0 || o3tl::make_unsigned(nTabNumber) <
maTabData.size();
+}
+
void ScViewData::UpdateCurrentTab()
{
- assert(0 <= mnTabNumber && o3tl::make_unsigned(mnTabNumber) <
maTabData.size());
+ assert(IsValidTabNumber(GetTabNumber()));
+
pThisTab = maTabData[mnTabNumber].get();
while (!pThisTab)
{
@@ -894,7 +900,8 @@ void ScViewData::InsertTabs( SCTAB nTab, SCTAB nNewSheets )
void ScViewData::DeleteTab( SCTAB nTab )
{
- assert(nTab < static_cast<SCTAB>(maTabData.size()));
+ assert(IsValidTabNumber(nTab));
+
maTabData.erase(maTabData.begin() + nTab);
if (o3tl::make_unsigned(GetTabNumber()) >= maTabData.size())
@@ -1090,7 +1097,7 @@ void ScViewData::SetZoom( const Fraction& rNewX, const
Fraction& rNewY, bool bAl
void ScViewData::SetShowGrid( bool bShow )
{
CreateSelectedTabData();
- maTabData[CurrentTabForData()]->bShowGrid = bShow;
+ maTabData[GetTabNumber()]->bShowGrid = bShow;
}
void ScViewData::RefreshZoom()
@@ -1411,7 +1418,7 @@ SCROW ScViewData::GetPosY( ScVSplitPos eWhich, SCTAB
nForTab ) const
ScViewDataTable* ScViewData::FetchTableData(SCTAB nTabIndex) const
{
- if (!ValidTab(nTabIndex) || (nTabIndex >=
static_cast<SCTAB>(maTabData.size())))
+ if (!ValidTab(nTabIndex) || !IsValidTabNumber(nTabIndex))
return nullptr;
ScViewDataTable* pRet = maTabData[nTabIndex].get();
SAL_WARN_IF(!pRet, "sc.viewdata", "ScViewData::FetchTableData: hidden
sheet = " << nTabIndex);
@@ -2429,12 +2436,12 @@ Point ScViewData::GetScrPos( SCCOL nWhereX, SCROW
nWhereY, ScSplitPos eWhich,
}
if (nForTab == -1)
- nForTab = CurrentTabForData();
- bool bForCurTab = (nForTab == CurrentTabForData());
+ nForTab = GetTabNumber();
+ bool bForCurTab = (nForTab == GetTabNumber());
if (!bForCurTab && (!ValidTab(nForTab) || (nForTab >=
static_cast<SCTAB>(maTabData.size()))))
{
SAL_WARN("sc.viewdata", "ScViewData::GetScrPos : invalid nForTab = "
<< nForTab);
- nForTab = CurrentTabForData();
+ nForTab = GetTabNumber();
bForCurTab = true;
}
@@ -4070,7 +4077,7 @@ bool ScViewData::IsOle() const
bool ScViewData::UpdateFixX( SCTAB nTab ) // true = value changed
{
if (!ValidTab(nTab)) // Default
- nTab = CurrentTabForData(); // current table
+ nTab = GetTabNumber(); // current table
if (!pView || maTabData[nTab]->eHSplitMode != SC_SPLIT_FIX)
return false;
@@ -4094,7 +4101,7 @@ bool ScViewData::UpdateFixX( SCTAB nTab ) // true = value
changed
if (nNewPos != maTabData[nTab]->nHSplitPos)
{
maTabData[nTab]->nHSplitPos = nNewPos;
- if (nTab == CurrentTabForData())
+ if (nTab == GetTabNumber())
RecalcPixPos(); // should not be needed
return true;
}
@@ -4105,7 +4112,7 @@ bool ScViewData::UpdateFixX( SCTAB nTab ) // true = value
changed
bool ScViewData::UpdateFixY( SCTAB nTab ) // true = value changed
{
if (!ValidTab(nTab)) // Default
- nTab = CurrentTabForData(); // current table
+ nTab = GetTabNumber(); // current table
if (!pView || maTabData[nTab]->eVSplitMode != SC_SPLIT_FIX)
return false;
@@ -4129,7 +4136,7 @@ bool ScViewData::UpdateFixY( SCTAB nTab ) // true = value
changed
if (nNewPos != maTabData[nTab]->nVSplitPos)
{
maTabData[nTab]->nVSplitPos = nNewPos;
- if (nTab == CurrentTabForData())
+ if (nTab == GetTabNumber())
RecalcPixPos(); // should not be needed
return true;
}
@@ -4277,7 +4284,7 @@ bool ScViewData::SetLOKSheetFreezeIndex(const SCCOLROW
nFreezeIndex, bool bIsCol
{
nForTab = CurrentTabForData();
}
- else if (!ValidTab(nForTab) || (nForTab >=
static_cast<SCTAB>(maTabData.size())))
+ else if (!ValidTab(nForTab) || !IsValidTabNumber(nForTab))
{
SAL_WARN("sc.viewdata", "ScViewData::SetLOKSheetFreezeIndex : invalid
nForTab = " << nForTab);
return false;
