tdf#47479: Do not modify range of sheet / column / row in notifications

Change-Id: I2c5afa5397e3a98806908a2f73f8d654e83fb890
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/184889
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <[email protected]>

Author: mikekaganski

sc/inc/cellsuno.hxx

@@ -227,6 +227,9 @@ protected:
     SC_DLLPUBLIC virtual void SetOnePropertyValue( const SfxItemPropertyMapEntry* pEntry,
                                                 const css::uno::Any& aValue );
 
+    ScRangeList& AccessRanges() { return aRanges; }
+    SC_DLLPUBLIC virtual void AdjustUpdatedRanges(UpdateRefMode mode);
+
 public:
                             ScCellRangesBase(ScDocShell* pDocSh, const ScRange& rR);
                             ScCellRangesBase(ScDocShell* pDocSh, ScRangeList aR);
@@ -935,6 +938,9 @@ public:
     virtual OUString SAL_CALL getImplementationName() override;
     virtual sal_Bool SAL_CALL supportsService( const OUString& ServiceName ) override;
     virtual css::uno::Sequence< OUString > SAL_CALL getSupportedServiceNames() override;
+
+protected:
+    void AdjustUpdatedRanges(UpdateRefMode mode) override;
 };
 
 using ScTableColumnObj_BASE = cppu::ImplInheritanceHelper<ScCellRangeObj,
@@ -966,6 +972,9 @@ public:
     virtual OUString SAL_CALL getImplementationName() override;
     virtual sal_Bool SAL_CALL supportsService( const OUString& ServiceName ) override;
     virtual css::uno::Sequence< OUString > SAL_CALL getSupportedServiceNames() override;
+
+protected:
+    void AdjustUpdatedRanges(UpdateRefMode mode) override;
 };
 
 class ScTableRowObj final : public ScCellRangeObj
@@ -991,6 +1000,9 @@ public:
     virtual OUString SAL_CALL getImplementationName() override;
     virtual sal_Bool SAL_CALL supportsService( const OUString& ServiceName ) override;
     virtual css::uno::Sequence< OUString > SAL_CALL getSupportedServiceNames() override;
+
+protected:
+    void AdjustUpdatedRanges(UpdateRefMode mode) override;
 };
 
 class ScCellsObj final : public cppu::WeakImplHelper<

sc/qa/extras/macros-test.cxx

@@ -17,8 +17,11 @@
 #include <document.hxx>
 #include <scitems.hxx>
 
+#include <com/sun/star/sheet/XCellRangeAddressable.hpp>
+#include <com/sun/star/sheet/XCellRangeMovement.hpp>
 #include <com/sun/star/sheet/XFunctionAccess.hpp>
 #include <com/sun/star/sheet/XSpreadsheet.hpp>
+#include <com/sun/star/table/XColumnRowRange.hpp>
 
 #include <com/sun/star/script/XLibraryContainerPassword.hpp>
 #include <com/sun/star/drawing/XDrawPageSupplier.hpp>
@@ -919,6 +922,60 @@ CPPUNIT_TEST_FIXTURE(ScMacrosTest, testTdf159412)
     CPPUNIT_ASSERT_EQUAL(u"1 Long/2 Double"_ustr, aReturnValue);
 }
 
+CPPUNIT_TEST_FIXTURE(ScMacrosTest, testTdf47479)
+{
+    createScDoc();
+    uno::Reference<sheet::XSpreadsheetDocument> xDoc(mxComponent, UNO_QUERY_THROW);
+    auto xSheets = xDoc->getSheets().queryThrow<container::XIndexAccess>();
+    auto xSheet = xSheets->getByIndex(0).queryThrow<sheet::XCellRangeMovement>();
+    auto xSheetAddressable = xSheet.queryThrow<sheet::XCellRangeAddressable>();
+    auto xColRowRange = xSheet.queryThrow<table::XColumnRowRange>();
+    auto xColAddressable
+        = xColRowRange->getColumns()->getByIndex(1).queryThrow<sheet::XCellRangeAddressable>();
+    auto xRowAddressable
+        = xColRowRange->getRows()->getByIndex(1).queryThrow<sheet::XCellRangeAddressable>();
+    css::table::CellRangeAddress origSheetRange = xSheetAddressable->getRangeAddress();
+    css::table::CellRangeAddress origColRange = xColAddressable->getRangeAddress();
+    css::table::CellRangeAddress origRowRange = xRowAddressable->getRangeAddress();
+    css::table::CellRangeAddress addressToRemove(origSheetRange.Sheet, 1, 1, 1, 1);
+
+    xSheet->removeRange(addressToRemove, css::sheet::CellDeleteMode_UP);
+
+    auto currentRange = xSheetAddressable->getRangeAddress();
+    CPPUNIT_ASSERT_EQUAL(origSheetRange.Sheet, currentRange.Sheet);
+    CPPUNIT_ASSERT_EQUAL(origSheetRange.StartColumn, currentRange.StartColumn);
+    CPPUNIT_ASSERT_EQUAL(origSheetRange.StartRow, currentRange.StartRow);
+    CPPUNIT_ASSERT_EQUAL(origSheetRange.EndColumn, currentRange.EndColumn);
+    // Without the fix, this would fail with
+    // - Expected: 1048575
+    // - Actual  : 0
+    CPPUNIT_ASSERT_EQUAL(origSheetRange.EndRow, currentRange.EndRow);
+
+    xSheet->removeRange(addressToRemove, css::sheet::CellDeleteMode_LEFT);
+
+    currentRange = xColAddressable->getRangeAddress();
+    CPPUNIT_ASSERT_EQUAL(origColRange.Sheet, currentRange.Sheet);
+    CPPUNIT_ASSERT_EQUAL(origColRange.StartColumn, currentRange.StartColumn);
+    // Without the fix, this would fail with
+    // - Expected: 0
+    // - Actual  : 2
+    CPPUNIT_ASSERT_EQUAL(origColRange.StartRow, currentRange.StartRow);
+    CPPUNIT_ASSERT_EQUAL(origColRange.EndColumn, currentRange.EndColumn);
+    CPPUNIT_ASSERT_EQUAL(origColRange.EndRow, currentRange.EndRow);
+
+    xSheet->removeRange(origColRange, css::sheet::CellDeleteMode_UP);
+
+    currentRange = xRowAddressable->getRangeAddress();
+    CPPUNIT_ASSERT_EQUAL(origRowRange.Sheet, currentRange.Sheet);
+    // Without the fix, this would fail with
+    // - Expected: 0
+    // - Actual  : 2
+    CPPUNIT_ASSERT_EQUAL(origRowRange.StartColumn, currentRange.StartColumn);
+    CPPUNIT_ASSERT_EQUAL(origRowRange.StartRow, currentRange.StartRow);
+    CPPUNIT_ASSERT_EQUAL(origRowRange.EndColumn, currentRange.EndColumn);
+    CPPUNIT_ASSERT_EQUAL(origRowRange.EndRow, currentRange.EndRow);
+}
+
 ScMacrosTest::ScMacrosTest()
       : ScModelTestBase(u"/sc/qa/extras/testdocuments"_ustr)
 {

sc/source/ui/unoobj/cellsuno.cxx

@@ -1475,6 +1475,8 @@ const ScMarkData* ScCellRangesBase::GetMarkData()
     return pMarkData.get();
 }
 
+void ScCellRangesBase::AdjustUpdatedRanges(UpdateRefMode) {}
+
 void ScCellRangesBase::Notify( SfxBroadcaster&, const SfxHint& rHint )
 {
     const SfxHintId nId = rHint.GetId();
@@ -1546,18 +1548,7 @@ void ScCellRangesBase::Notify( SfxBroadcaster&, const SfxHint& rHint )
         if ( aRanges.UpdateReference( pRefHint->GetMode(), &rDoc, pRefHint->GetRange(),
                                     pRefHint->GetDx(), pRefHint->GetDy(), pRefHint->GetDz() ) )
         {
-            if (  pRefHint->GetMode() == URM_INSDEL
-               && aRanges.size() == 1
-               && dynamic_cast<ScTableSheetObj*>(this)
-               )
-            {
-                // #101755#; the range size of a sheet does not change
-                ScRange & rR = aRanges.front();
-                rR.aStart.SetCol(0);
-                rR.aStart.SetRow(0);
-                rR.aEnd.SetCol(rDoc.MaxCol());
-                rR.aEnd.SetRow(rDoc.MaxRow());
-            }
+            AdjustUpdatedRanges(pRefHint->GetMode());
             RefChanged();
 
             // any change of the range address is broadcast to value (modify) listeners
@@ -6392,6 +6383,17 @@ SCTAB ScTableSheetObj::GetTab_Impl() const
     return 0;
 }
 
+void ScTableSheetObj::AdjustUpdatedRanges(UpdateRefMode mode)
+{
+    if (mode == URM_INSDEL)
+    {
+        ScRangeList& rRanges = AccessRanges();
+        // #101755#, tdf#47479: the range of a sheet does not change
+        rRanges.RemoveAll();
+        rRanges.push_back(GetRange());
+    }
+}
+
 // former XSheet
 
 uno::Reference<table::XTableCharts> SAL_CALL ScTableSheetObj::getCharts()
@@ -8195,6 +8197,17 @@ const SfxItemPropertyMap& ScTableColumnObj::GetItemPropertyMap()
     return pColPropSet->getPropertyMap();
 }
 
+void ScTableColumnObj::AdjustUpdatedRanges(UpdateRefMode mode)
+{
+    if (mode == URM_INSDEL)
+    {
+        ScRangeList& rRanges = AccessRanges();
+        // tdf#47479: the range of a column does not change
+        rRanges.RemoveAll();
+        rRanges.push_back(GetRange());
+    }
+}
+
 ScTableRowObj::ScTableRowObj(ScDocShell* pDocSh, SCROW nRow, SCTAB nTab) :
     ScCellRangeObj( pDocSh, ScRange(0,nRow,nTab, pDocSh->GetDocument().MaxCol(),nRow,nTab) ),
     pRowPropSet(lcl_GetRowPropertySet())
@@ -8344,6 +8357,17 @@ const SfxItemPropertyMap& ScTableRowObj::GetItemPropertyMap()
     return pRowPropSet->getPropertyMap();
 }
 
+void ScTableRowObj::AdjustUpdatedRanges(UpdateRefMode mode)
+{
+    if (mode == URM_INSDEL)
+    {
+        ScRangeList& rRanges = AccessRanges();
+        // tdf#47479: the range of a row does not change
+        rRanges.RemoveAll();
+        rRanges.push_back(GetRange());
+    }
+}
+
 ScCellsObj::ScCellsObj(ScDocShell* pDocSh, ScRangeList aR) :
     pDocShell( pDocSh ),
     aRanges(std::move( aR ))