sc: Preserve #SPILL! state through XLSX and ODS import

A dynamic-array formula saved while its spill range was blocked came back from save+load with the blocker overwritten by reference cells, because the import path materialised the matrix to its declared range without checking whether the blocking cells were still there. Push a "saved as #SPILL!" flag through the XLSX and ODS array import paths and have setMatrixCells preserve the blockers when set. Clearing the blocker via the normal change operations then expands the matrix to its full result. Also extract a shared ScDocument::IsMatrixSpillBlocked helper that checks if a range has a blocker. The XLSX import detects "#SPILL!" on the array master's cached <v> element. The ODS import detects it via GetFormulaErrorConstant on the cached string result. Change-Id: Id86f71c5fbbfc9a469ce93fd57ff534069d0ab16 Signed-off-by: Tomaž Vajngerl <tomaz.vajngerl@collabora.co.uk> Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/2648 Reviewed-by: Miklos Vajna <vmiklos@collabora.com> Tested-by: Jenkins CPCI <releng@collaboraoffice.com> (cherry picked from commit e4476c8072af0d825578f5222935718a159bb383) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/205755 Tested-by: Jenkins Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
2026-05-27 18:20:18 +02:00
parent 784fe2c830
commit 60f1c75e21
16 changed files with 175 additions and 48 deletions
@@ -1328,6 +1328,12 @@ public:
                                        const ScTokenArray* p = nullptr,
                                        const formula::FormulaGrammar::Grammar = formula::FormulaGrammar::GRAM_DEFAULT,
                                        bool bCheckForSpill = false);
+    /** Returns true if any cell inside input range (rRange) other than the cells
+        already covered by the declared nDeclCols x nDeclRows sub-range holds any
+        data. */
+    SC_DLLPUBLIC bool IsMatrixSpillBlocked(const ScRange& rRange,
+                                           SCCOL nDeclCols = 1,
+                                           SCROW nDeclRows = 1) const;
    /** Resize an existing matrix formula at input position to the given dimensions.
        Updates the matrix cell token, creates reference cells in newly covered
        positions, and clears cells that fall outside the new range. */
@@ -112,8 +112,11 @@ public:
    void setFormulaCell(const ScAddress& rPos, std::unique_ptr<ScTokenArray> pArray);
    void setFormulaCell(const ScAddress& rPos, ScFormulaCell* pCell);

+    /// When the saved file recorded the matrix in a #SPILL! state, pass
+    /// bCheckForSpill=true.
    void setMatrixCells(
-        const ScRange& rRange, const ScTokenArray& rArray, formula::FormulaGrammar::Grammar eGrammar);
+        const ScRange& rRange, const ScTokenArray& rArray, formula::FormulaGrammar::Grammar eGrammar,
+        bool bCheckForSpill = false);

    void setTableOpCells(const ScRange& rRange, const ScTabOpParam& rParam);

@@ -330,6 +330,17 @@ void ScModelTestBase::insertArrayToCell(const OUString& rCell, std::u16string_vi
    Scheduler::ProcessEventsToIdle();
 }

+void ScModelTestBase::clearCell(const OUString& rCell)
+{
+    goToCell(rCell);
+
+    // Press DELETE on a selected cell
+    ScModelObj* pModelObj = comphelper::getFromUnoTunnel<ScModelObj>(mxComponent);
+    pModelObj->postKeyEvent(LOK_KEYEVENT_KEYINPUT, 0, awt::Key::DELETE);
+    pModelObj->postKeyEvent(LOK_KEYEVENT_KEYUP, 0, awt::Key::DELETE);
+    Scheduler::ProcessEventsToIdle();
+}
+
 void ScModelTestBase::insertNewSheet(const ScDocument& rDoc)
 {
    sal_Int16 nTabs = rDoc.GetTableCount();
@@ -178,6 +178,7 @@ public:
    void typeString(std::u16string_view rStr);
    void insertStringToCell(const OUString& rCell, std::u16string_view rStr);
    void insertArrayToCell(const OUString& rCell, std::u16string_view rStr);
+    void clearCell(const OUString& rCell);
    void insertNewSheet(const ScDocument& rDoc);
    void executeAutoSum();
    void selectObjectByName(std::u16string_view rObjName);
@@ -1747,6 +1747,63 @@ CPPUNIT_TEST_FIXTURE(ScExportTest2, testSpillErrorRoundtripODS)
    CPPUNIT_ASSERT_EQUAL(sal_Int32(FormulaError::Spill), sal_Int32(pFormulaCell->GetErrCode()));
 }

+CPPUNIT_TEST_FIXTURE(ScExportTest2, testArrayFormulaSpillRoundtripXLSX)
+{
+    // UNIQUE formula blocked by another cell.
+    // The formula text, the #SPILL! error, and the blocker all need to be present
+    // after round-trip. Clearing the blocker must expand the matrix result.
+
+    createScDoc();
+
+    // Source data
+    insertStringToCell(u"B1"_ustr, u"10");
+    insertStringToCell(u"B2"_ustr, u"20");
+    insertStringToCell(u"B3"_ustr, u"30");
+    insertStringToCell(u"B4"_ustr, u"40");
+
+    // Blocker at A2
+    insertStringToCell(u"A2"_ustr, u"blocker");
+
+    // UNIQUE formula entered
+    insertArrayToCell(u"A1"_ustr, u"=UNIQUE(B1:B4)");
+
+    {
+        ScDocument* pDocument = getScDoc();
+        ScFormulaCell* pFormulaCell = pDocument->GetFormulaCell(ScAddress(0, 0, 0));
+        CPPUNIT_ASSERT_MESSAGE("UNIQUE result should be #SPILL!", pFormulaCell != nullptr);
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(FormulaError::Spill), sal_Int32(pFormulaCell->GetErrCode()));
+    }
+
+    saveAndReload(TestFilter::XLSX);
+
+    ScDocument* pDocument = getScDoc();
+
+    // Blocker text must survive.
+    CPPUNIT_ASSERT_EQUAL(u"blocker"_ustr, pDocument->GetString(ScAddress(0, 1, 0)));
+
+    // The formula cell must still be a matrix formula carrying #SPILL!
+    ScFormulaCell* pFormulaCell = pDocument->GetFormulaCell(ScAddress(0, 0, 0));
+    CPPUNIT_ASSERT_MESSAGE("Expected a formula cell at A1", pFormulaCell != nullptr);
+    CPPUNIT_ASSERT_EQUAL(ScMatrixMode::Formula, pFormulaCell->GetMatrixFlag());
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(FormulaError::Spill), sal_Int32(pFormulaCell->GetErrCode()));
+    CPPUNIT_ASSERT_MESSAGE("UNIQUE token array must keep its dynamic array flag",
+                           pFormulaCell->GetCode()->HasDynamicArrayFunction());
+
+    // Clear the blocker - dynamic array must expand.
+    clearCell(u"A2"_ustr);
+
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(FormulaError::NONE), sal_Int32(pFormulaCell->GetErrCode()));
+    SCCOL nCols = 0;
+    SCROW nRows = 0;
+    pFormulaCell->GetMatColsRows(nCols, nRows);
+    CPPUNIT_ASSERT_EQUAL(SCCOL(1), nCols);
+    CPPUNIT_ASSERT_EQUAL(SCROW(4), nRows);
+    CPPUNIT_ASSERT_EQUAL(10.0, pDocument->GetValue(ScAddress(0, 0, 0)));
+    CPPUNIT_ASSERT_EQUAL(20.0, pDocument->GetValue(ScAddress(0, 1, 0)));
+    CPPUNIT_ASSERT_EQUAL(30.0, pDocument->GetValue(ScAddress(0, 2, 0)));
+    CPPUNIT_ASSERT_EQUAL(40.0, pDocument->GetValue(ScAddress(0, 3, 0)));
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();

 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
@@ -252,6 +252,23 @@ bool ScDocument::Solver(SCCOL nFCol, SCROW nFRow, SCTAB nFTab,
    return bRet;
 }

+bool ScDocument::IsMatrixSpillBlocked(const ScRange& rRange,
+                                      SCCOL nDeclCols, SCROW nDeclRows) const
+{
+    const ScAddress& rOrigin = rRange.aStart;
+    for (SCCOL nCol = rRange.aStart.Col(); nCol <= rRange.aEnd.Col(); ++nCol)
+    {
+        for (SCROW nRow = rRange.aStart.Row(); nRow <= rRange.aEnd.Row(); ++nRow)
+        {
+            if (nCol < rOrigin.Col() + nDeclCols && nRow < rOrigin.Row() + nDeclRows)
+                continue; // inside the already-declared sub-range
+            if (const_cast<ScDocument*>(this)->HasData(nCol, nRow, rOrigin.Tab()))
+                return true;
+        }
+    }
+    return false;
+}
+
 void ScDocument::InsertMatrixFormula(SCCOL nCol1, SCROW nRow1,
                                     SCCOL nCol2, SCROW nRow2,
                                     const ScMarkData& rMark,
@@ -321,15 +338,10 @@ void ScDocument::InsertMatrixFormula(SCCOL nCol1, SCROW nRow1,
                break;
            if (!FetchTable(nTab))
                continue;
-            for (SCCOL nCol = nCol1; nCol <= nCol2 && !bSpillBlocked; ++nCol)
+            if (IsMatrixSpillBlocked(ScRange(nCol1, nRow1, nTab, nCol2, nRow2, nTab)))
            {
-                for (SCROW nRow = nRow1; nRow <= nRow2 && !bSpillBlocked; ++nRow)
-                {
-                    if (nCol == nCol1 && nRow == nRow1)
-                        continue; // skip the origin cell
-                    if (HasData(nCol, nRow, nTab))
-                        bSpillBlocked = true;
-                }
+                bSpillBlocked = true;
+                break;
            }
        }
        if (bSpillBlocked)
@@ -399,7 +399,8 @@ void ScDocumentImport::setFormulaCell(const ScAddress& rPos, ScFormulaCell* pCel
 }

 void ScDocumentImport::setMatrixCells(
-    const ScRange& rRange, const ScTokenArray& rArray, formula::FormulaGrammar::Grammar eGram)
+    const ScRange& rRange, const ScTokenArray& rArray, formula::FormulaGrammar::Grammar eGram,
+    bool bCheckForSpill)
 {
    const ScAddress& rBasePos = rRange.aStart;

@@ -429,6 +430,21 @@ void ScDocumentImport::setMatrixCells(
    pCell->SetMatColsRows(
        rRange.aEnd.Col()-rRange.aStart.Col()+1, rRange.aEnd.Row()-rRange.aStart.Row()+1);

+    // The file was saved with a #SPILL! error. If the matrix range still has
+    // data in any non-origin cell those are real blockers from the saved
+    // state. Keep the blockers and the spill error rather than materialising
+    // reference cells over them.
+    if (bCheckForSpill && (rRange.aEnd.Col() > rRange.aStart.Col()
+                           || rRange.aEnd.Row() > rRange.aStart.Row())
+        && mpImpl->mrDoc.IsMatrixSpillBlocked(rRange))
+    {
+        // Collapse the declared dimensions back to 1x1
+        pCell->SetMatColsRows(1, 1);
+        pCell->SetResultError(FormulaError::Spill);
+        mpImpl->mrDoc.MarkFormulaSpilled(rBasePos);
+        return;
+    }
+
    // Set the reference cells.
    ScSingleRefData aRefData;
    aRefData.InitFlags();
@@ -2331,21 +2331,10 @@ void ScFormulaCell::InterpretTail( ScInterpreterContext& rContext, ScInterpretTa
                    }
                    else
                    {
-                        for (SCCOL nColumn = aPos.Col();
-                             nColumn <= nSpillEndCol && !bSpillBlocked; ++nColumn)
-                        {
-                            for (SCROW nRow = aPos.Row();
-                                 nRow <= nSpillEndRow && !bSpillBlocked; ++nRow)
-                            {
-                                if (nColumn < aPos.Col() + nDeclCols
-                                    && nRow < aPos.Row() + nDeclRows)
-                                {
-                                    continue; // inside declared area
-                                }
-                                if (rDocument.HasData(nColumn, nRow, aPos.Tab()))
-                                    bSpillBlocked = true;
-                            }
-                        }
+                        bSpillBlocked = rDocument.IsMatrixSpillBlocked(
+                            ScRange(aPos.Col(), aPos.Row(), aPos.Tab(),
+                                    nSpillEndCol, nSpillEndRow, aPos.Tab()),
+                            nDeclCols, nDeclRows);
                    }
                    if (bSpillBlocked)
                    {
@@ -59,7 +59,18 @@ public:
    {
        TokenAddressItem maTokenAndAddress;
        ScRange maRange;
-        TokenRangeAddressItem( TokenAddressItem aTokenAndAddress, const ScRange& rRange ) : maTokenAndAddress(std::move( aTokenAndAddress )), maRange( rRange ) {}
+
+        /// File was saved with a #SPILL! error on the master cell. Triggers
+        /// blocker preservation so reference cells aren't materialised over
+        /// real user data sitting in the matrix range.
+        bool mbCachedSpill;
+
+        TokenRangeAddressItem(TokenAddressItem aTokenAndAddress, const ScRange& rRange,
+                              bool bCachedSpill = false)
+            : maTokenAndAddress(std::move(aTokenAndAddress))
+            , maRange(rRange)
+            , mbCachedSpill(bCachedSpill) 
+        {}
    };

    struct FormulaValue
@@ -106,9 +117,10 @@ public:
    void setCellFormulaValue(
        const ScAddress& rAddress, const OUString& rValueStr, sal_Int32 nCellType );

-    void                setCellArrayFormula( const ScRange& rRangeAddress,
-                                             const ScAddress& rTokenAddress,
-                                             const OUString& );
+    void setCellArrayFormula(const ScRange& rRangeAddress,
+                             const ScAddress& rTokenAddress,
+                             const OUString&,
+                             bool bCachedSpill = false);

    void                createSharedFormulaMapEntry( const ScAddress& rAddress,
                                                     sal_Int32 nSharedId, const OUString& rTokens );
@@ -290,7 +290,7 @@ public:
        const ScAddress& rAddr, sal_Int32 nSharedId,
        const OUString& rCellValue, sal_Int32 nValueType );

-    void                setCellArrayFormula( const ScRange& rRangeAddress, const ScAddress& rTokenAddress, const OUString& rTokenStr );
+    void setCellArrayFormula(const ScRange& rRangeAddress, const ScAddress& rTokenAddress, const OUString& rTokenStr, bool bCachedSpill = false);

    void createSharedFormulaMapEntry(
        const ScAddress& rAddress,
@@ -302,7 +302,11 @@ void applyArrayFormulas(
        aComp.SetExternalLinks(rExternalLinks);
        std::unique_ptr<ScTokenArray> pArray(aComp.CompileString(rAddressItem.maTokenAndAddress.maTokenStr));
        if (pArray)
-            rDoc.setMatrixCells(rAddressItem.maRange, *pArray, formula::FormulaGrammar::GRAM_OOXML);
+        {
+            rDoc.setMatrixCells(rAddressItem.maRange, *pArray,
+                                formula::FormulaGrammar::GRAM_OOXML,
+                                rAddressItem.mbCachedSpill);
+        }
    }
 }

@@ -522,12 +526,12 @@ void FormulaBuffer::setCellFormula(
    maSharedFormulaIds[rAddress.Tab()].emplace_back(rAddress, nSharedId, rCellValue, nValueType);
 }

-void FormulaBuffer::setCellArrayFormula( const ScRange& rRangeAddress, const ScAddress& rTokenAddress, const OUString& rTokenStr )
+void FormulaBuffer::setCellArrayFormula( const ScRange& rRangeAddress, const ScAddress& rTokenAddress, const OUString& rTokenStr, bool bCachedSpill)
 {

    TokenAddressItem tokenPair( rTokenStr, rTokenAddress );
    assert( rRangeAddress.aStart.Tab() >= 0 && o3tl::make_unsigned(rRangeAddress.aStart.Tab()) < maCellArrayFormulas.size() );
-    maCellArrayFormulas[ rRangeAddress.aStart.Tab() ].emplace_back( tokenPair, rRangeAddress );
+    maCellArrayFormulas[rRangeAddress.aStart.Tab()].emplace_back(tokenPair, rRangeAddress, bCachedSpill);
 }

 void FormulaBuffer::setCellFormulaValue(
@@ -171,7 +171,14 @@ void SheetDataContext::onEndElement()
        case XML_array:
            if( mbValidRange && maFmlaData.isValidArrayRef( maCellData.maCellAddr ) )
            {
-                setCellArrayFormula( maFmlaData.maFormulaRef, maCellData.maCellAddr, maFormulaStr );
+                // The cached value #SPILL! on an XML_e cell signals
+                // that the file was saved while the array formula's spill
+                // range was blocked. Transport that through so the import path
+                // preserves any blocker cells in the matrix range instead of
+                // overwriting them with reference cells.
+                const bool bCachedSpill = (maCellData.mnCellType == XML_e
+                                           && maCellValue == u"#SPILL!");
+                setCellArrayFormula(maFmlaData.maFormulaRef, maCellData.maCellAddr, maFormulaStr, bCachedSpill);
            }
            // set cell formatting, but do not set result as cell value
            mrSheetData.setBlankCell( maCellData );
@@ -1656,9 +1656,9 @@ void WorksheetHelper::setCellFormula(
    getFormulaBuffer().setCellFormula(rAddr, nSharedId, rCellValue, nValueType);
 }

-void WorksheetHelper::setCellArrayFormula( const ScRange& rRangeAddress, const ScAddress& rTokenAddress, const OUString& rTokenStr )
+void WorksheetHelper::setCellArrayFormula(const ScRange& rRangeAddress, const ScAddress& rTokenAddress, const OUString& rTokenStr, bool bCachedSpill)
 {
-    getFormulaBuffer().setCellArrayFormula( rRangeAddress,  rTokenAddress, rTokenStr );
+    getFormulaBuffer().setCellArrayFormula(rRangeAddress,  rTokenAddress, rTokenStr, bCachedSpill);
 }

 void WorksheetHelper::createSharedFormulaMapEntry(
@@ -1442,13 +1442,20 @@ void ScXMLTableRowCellContext::AddFormulaCell( const ScAddress& rCellPos )
        {
            if (nMatrixCols > 0 && nMatrixRows > 0)
            {
-                //matrix cells are put in the document, but we must set the
-                //value/text of each matrix cell later
+                // Matrix cells are put in the document, but we must set the
+                // value/text of each matrix cell later.
+                // The file recorded the array's master cell with a #SPILL!
+                // error result. Transport that through so any blocker cell still
+                // sitting in the matrix range is preserved instead of being
+                // replaced by a reference cell.
+                const bool bCachedSpill = (bFormulaTextResult && maStringValue
+                                           && GetScImport().GetFormulaErrorConstant(*maStringValue)
+                                              == FormulaError::Spill);
                rXMLImport.GetTables().AddMatrixRange(
                        rCellPos.Col(), rCellPos.Row(),
                        std::min<SCCOL>(rCellPos.Col() + nMatrixCols - 1, pDoc->MaxCol()),
                        std::min<SCROW>(rCellPos.Row() + nMatrixRows - 1, pDoc->MaxRow()),
-                        maFormula->first, maFormula->second, eGrammar);
+                        maFormula->first, maFormula->second, eGrammar, bCachedSpill);

                // Set the value/text of the top-left matrix position in its
                // cached result.  For import, we only need to set the correct
@@ -260,7 +260,8 @@ void ScMyTables::AddOLE(const uno::Reference <drawing::XShape>& rShape,

 void ScMyTables::AddMatrixRange(
        const SCCOL nStartColumn, const SCROW nStartRow, const SCCOL nEndColumn, const SCROW nEndRow,
-        const OUString& rFormula, const OUString& rFormulaNmsp, const formula::FormulaGrammar::Grammar eGrammar)
+        const OUString& rFormula, const OUString& rFormulaNmsp, const formula::FormulaGrammar::Grammar eGrammar,
+        bool bCachedSpill)
 {
    OSL_ENSURE(nEndRow >= nStartRow, "wrong row order");
    OSL_ENSURE(nEndColumn >= nStartColumn, "wrong column order");
@@ -275,7 +276,7 @@ void ScMyTables::AddMatrixRange(
    ScTokenArray aCode(rDoc.getDoc());
    aCode.AssignXMLString( rFormula,
            ((eGrammar == formula::FormulaGrammar::GRAM_EXTERNAL) ? rFormulaNmsp : OUString()));
-    rDoc.setMatrixCells(aScRange, aCode, eGrammar);
+    rDoc.setMatrixCells(aScRange, aCode, eGrammar, bCachedSpill);
    rDoc.getDoc().IncXMLImportedFormulaCount( rFormula.getLength() );
 }

@@ -97,13 +97,14 @@ public:
    void                                AddOLE(const css::uno::Reference <css::drawing::XShape>& rShape,
                                               const OUString &rRangeList);

-    void                                AddMatrixRange( const SCCOL nStartColumn,
-                                            const SCROW nStartRow,
-                                            const SCCOL nEndColumn,
-                                            const SCROW nEndRow,
-                                            const OUString& rFormula,
-                                            const OUString& rFormulaNmsp,
-                                            const formula::FormulaGrammar::Grammar );
+    void AddMatrixRange(const SCCOL nStartColumn,
+                        const SCROW nStartRow,
+                        const SCCOL nEndColumn,
+                        const SCROW nEndRow,
+                        const OUString& rFormula,
+                        const OUString& rFormulaNmsp,
+                        const formula::FormulaGrammar::Grammar,
+                        bool bCachedSpill = false);
    bool                                IsPartOfMatrix( const ScAddress& rScAddress) const;
 };