Hi there, I'd like to backport a fix from master to the libreoffice-3-3. The patch is attached, which is a cleaned up version of
http://cgit.freedesktop.org/libreoffice/calc/commit/?id=e98dd6643b15a156dbf3574343f3c076ad7ac277 The patch would likely not apply cleanly due to some BOOL to bool conversions I've been doing on master, but that's trivial to fix. This fixes the bug reported here http://openoffice.org/bugzilla/show_bug.cgi?id=116833 Here is some background on this. When Calc opens an existing ODS document, it stores the original XML contents, and re-uses it on export for unmodified documents. During editing, Calc invalidates the XML streams of modified sheets, and that info is used to determine whether to use the cached XML content or re-generate a new XML content from the document. The bug was that, when the reference of a cell on Sheet A is updated due to the relocation of the referenced cell on Sheet B, Calc would invalidate Sheet B but not Sheet A. So it re-uses the old XML stream for Sheet A which removes the reference update that's just been done on Sheet A. The solution is to properly identify cells whose references have been updated after moving the referenced cell, and invalidates the sheets they are on. Review would be appreciated. Kohei -- Kohei Yoshida, LibreOffice hacker, Calc <[email protected]>
commit e98dd6643b15a156dbf3574343f3c076ad7ac277 Author: Kohei Yoshida <[email protected]> Date: Mon Mar 7 18:40:10 2011 -0500 Properly invalidate cached sheet XML streams during reference update. Failure to invalidate sheet streams during reference update caused formula cross referenceing between sheets to totallly get borked. The bug was originally reported in i#116833. diff --git a/sc/inc/cell.hxx b/sc/inc/cell.hxx index 110c848..38e85ef 100644 --- a/sc/inc/cell.hxx +++ b/sc/inc/cell.hxx @@ -440,7 +440,7 @@ public: BOOL HasRelNameReference() const; BOOL HasColRowName() const; - void UpdateReference(UpdateRefMode eUpdateRefMode, + bool UpdateReference(UpdateRefMode eUpdateRefMode, const ScRange& r, SCsCOL nDx, SCsROW nDy, SCsTAB nDz, ScDocument* pUndoDoc = NULL, diff --git a/sc/inc/column.hxx b/sc/inc/column.hxx index 35d26b0..e4ab44b 100644 --- a/sc/inc/column.hxx +++ b/sc/inc/column.hxx @@ -285,7 +285,7 @@ public: void ResetChanged( SCROW nStartRow, SCROW nEndRow ); - void UpdateReference( UpdateRefMode eUpdateRefMode, SCCOL nCol1, SCROW nRow1, SCTAB nTab1, + bool UpdateReference( UpdateRefMode eUpdateRefMode, SCCOL nCol1, SCROW nRow1, SCTAB nTab1, SCCOL nCol2, SCROW nRow2, SCTAB nTab2, SCsCOL nDx, SCsROW nDy, SCsTAB nDz, ScDocument* pUndoDoc = NULL ); diff --git a/sc/source/core/data/cell2.cxx b/sc/source/core/data/cell2.cxx index a47dd14..06f53dc 100644 --- a/sc/source/core/data/cell2.cxx +++ b/sc/source/core/data/cell2.cxx @@ -825,11 +825,13 @@ BOOL ScFormulaCell::HasColRowName() const return (pCode->GetNextColRowName() != NULL); } -void ScFormulaCell::UpdateReference(UpdateRefMode eUpdateRefMode, +bool ScFormulaCell::UpdateReference(UpdateRefMode eUpdateRefMode, const ScRange& r, SCsCOL nDx, SCsROW nDy, SCsTAB nDz, ScDocument* pUndoDoc, const ScAddress* pUndoCellPos ) { + bool bCellStateChanged = false; + SCCOL nCol1 = r.aStart.Col(); SCROW nRow1 = r.aStart.Row(); SCTAB nTab1 = r.aStart.Tab(); @@ -858,6 +860,7 @@ void ScFormulaCell::UpdateReference(UpdateRefMode eUpdateRefMode, nCol = 0; else if ( nCol > MAXCOL ) nCol = MAXCOL; + bCellStateChanged = aPos.Col() != nCol; aPos.SetCol( nCol ); } } @@ -871,6 +874,7 @@ void ScFormulaCell::UpdateReference(UpdateRefMode eUpdateRefMode, nRow = 0; else if ( nRow > MAXROW ) nRow = MAXROW; + bCellStateChanged = aPos.Row() != nRow; aPos.SetRow( nRow ); } } @@ -885,6 +889,7 @@ void ScFormulaCell::UpdateReference(UpdateRefMode eUpdateRefMode, nTab = 0; else if ( nTab > nMaxTab ) nTab = nMaxTab; + bCellStateChanged = aPos.Tab() != nTab; aPos.SetTab( nTab ); } } @@ -932,6 +937,9 @@ void ScFormulaCell::UpdateReference(UpdateRefMode eUpdateRefMode, bRangeModified = false; bRefSizeChanged = false; } + + bCellStateChanged |= bValChanged; + if ( bOnRefMove ) bOnRefMove = (bValChanged || (aPos != aOldPos)); // Cell may reference itself, e.g. ocColumn, ocRow without parameter @@ -1118,6 +1126,7 @@ void ScFormulaCell::UpdateReference(UpdateRefMode eUpdateRefMode, delete pOld; } + return bCellStateChanged; } void ScFormulaCell::UpdateInsertTab(SCTAB nTable) diff --git a/sc/source/core/data/column.cxx b/sc/source/core/data/column.cxx index 9e13354..86415b3 100644 --- a/sc/source/core/data/column.cxx +++ b/sc/source/core/data/column.cxx @@ -1617,10 +1617,11 @@ void ScColumn::MoveTo(SCROW nStartRow, SCROW nEndRow, ScColumn& rCol) } -void ScColumn::UpdateReference( UpdateRefMode eUpdateRefMode, SCCOL nCol1, SCROW nRow1, SCTAB nTab1, +bool ScColumn::UpdateReference( UpdateRefMode eUpdateRefMode, SCCOL nCol1, SCROW nRow1, SCTAB nTab1, SCCOL nCol2, SCROW nRow2, SCTAB nTab2, SCsCOL nDx, SCsROW nDy, SCsTAB nDz, ScDocument* pUndoDoc ) { + bool bUpdated = false; if (pItems) { ScRange aRange( ScAddress( nCol1, nRow1, nTab1 ), @@ -1632,7 +1633,8 @@ void ScColumn::UpdateReference( UpdateRefMode eUpdateRefMode, SCCOL nCol1, SCROW { ScFormulaCell* pCell = (ScFormulaCell*) pItems[nIndex].pCell; if( pCell->GetCellType() == CELLTYPE_FORMULA) - pCell->UpdateReference( eUpdateRefMode, aRange, nDx, nDy, nDz, pUndoDoc ); + bUpdated |= pCell->UpdateReference( + eUpdateRefMode, aRange, nDx, nDy, nDz, pUndoDoc ); } } else @@ -1653,7 +1655,8 @@ void ScColumn::UpdateReference( UpdateRefMode eUpdateRefMode, SCCOL nCol1, SCROW ScBaseCell* pCell = pItems[i].pCell; if( pCell->GetCellType() == CELLTYPE_FORMULA) { - ((ScFormulaCell*)pCell)->UpdateReference( eUpdateRefMode, aRange, nDx, nDy, nDz, pUndoDoc ); + bUpdated |= ((ScFormulaCell*)pCell)->UpdateReference( + eUpdateRefMode, aRange, nDx, nDy, nDz, pUndoDoc ); if ( nRow != pItems[i].nRow ) Search( nRow, i ); // Listener removed/inserted? } @@ -1671,7 +1674,8 @@ void ScColumn::UpdateReference( UpdateRefMode eUpdateRefMode, SCCOL nCol1, SCROW // When deleting rows on several sheets, the formula's position may be updated with the first call, // so the undo position must be passed from here. ScAddress aUndoPos( nCol, nRow, nTab ); - ((ScFormulaCell*)pCell)->UpdateReference( eUpdateRefMode, aRange, nDx, nDy, nDz, pUndoDoc, &aUndoPos ); + bUpdated |= ((ScFormulaCell*)pCell)->UpdateReference( + eUpdateRefMode, aRange, nDx, nDy, nDz, pUndoDoc, &aUndoPos ); if ( nRow != pItems[i].nRow ) Search( nRow, i ); // Listener removed/inserted? } @@ -1679,6 +1683,7 @@ void ScColumn::UpdateReference( UpdateRefMode eUpdateRefMode, SCCOL nCol1, SCROW } } } + return bUpdated; } diff --git a/sc/source/core/data/table1.cxx b/sc/source/core/data/table1.cxx index f4e71bd..bd25fa3 100644 --- a/sc/source/core/data/table1.cxx +++ b/sc/source/core/data/table1.cxx @@ -1280,6 +1280,7 @@ void ScTable::UpdateReference( UpdateRefMode eUpdateRefMode, SCCOL nCol1, SCROW SCCOL nCol2, SCROW nRow2, SCTAB nTab2, SCsCOL nDx, SCsROW nDy, SCsTAB nDz, ScDocument* pUndoDoc, BOOL bIncludeDraw, bool bUpdateNoteCaptionPos ) { + bool bUpdated = false; SCCOL i; SCCOL iMax; if ( eUpdateRefMode == URM_COPY ) @@ -1293,8 +1294,8 @@ void ScTable::UpdateReference( UpdateRefMode eUpdateRefMode, SCCOL nCol1, SCROW iMax = MAXCOL; } for ( ; i<=iMax; i++) - aCol[i].UpdateReference( eUpdateRefMode, nCol1, nRow1, nTab1, nCol2, nRow2, nTab2, - nDx, nDy, nDz, pUndoDoc ); + bUpdated |= aCol[i].UpdateReference( + eUpdateRefMode, nCol1, nRow1, nTab1, nCol2, nRow2, nTab2, nDx, nDy, nDz, pUndoDoc ); if ( bIncludeDraw ) UpdateDrawRef( eUpdateRefMode, nCol1, nRow1, nTab1, nCol2, nRow2, nTab2, nDx, nDy, nDz, bUpdateNoteCaptionPos ); @@ -1379,6 +1380,9 @@ void ScTable::UpdateReference( UpdateRefMode eUpdateRefMode, SCCOL nCol1, SCROW PAINT_GRID ) ); } } + + if (bUpdated && IsStreamValid()) + SetStreamValid(false); } void ScTable::UpdateTranspose( const ScRange& rSource, const ScAddress& rDest, diff --git a/sc/source/core/data/table2.cxx b/sc/source/core/data/table2.cxx index 808f952..bc00fe8 100644 --- a/sc/source/core/data/table2.cxx +++ b/sc/source/core/data/table2.cxx @@ -177,6 +177,11 @@ void ScTable::InsertRow( SCCOL nStartCol, SCCOL nEndCol, SCROW nStartRow, SCSIZE DecRecalcLevel( false ); InvalidatePageBreaks(); + + if (IsStreamValid()) + // TODO: In the future we may want to check if the table has been + // really modified before setting the stream invalid. + SetStreamValid(false); } @@ -222,6 +227,11 @@ void ScTable::DeleteRow( SCCOL nStartCol, SCCOL nEndCol, SCROW nStartRow, SCSIZE DecRecalcLevel(); InvalidatePageBreaks(); + + if (IsStreamValid()) + // TODO: In the future we may want to check if the table has been + // really modified before setting the stream invalid. + SetStreamValid(false); } @@ -309,6 +319,11 @@ void ScTable::InsertCol( SCCOL nStartCol, SCROW nStartRow, SCROW nEndRow, SCSIZE DecRecalcLevel(); InvalidatePageBreaks(); + + if (IsStreamValid()) + // TODO: In the future we may want to check if the table has been + // really modified before setting the stream invalid. + SetStreamValid(false); } @@ -369,6 +384,11 @@ void ScTable::DeleteCol( SCCOL nStartCol, SCROW nStartRow, SCROW nEndRow, SCSIZE DecRecalcLevel(); InvalidatePageBreaks(); + + if (IsStreamValid()) + // TODO: In the future we may want to check if the table has been + // really modified before setting the stream invalid. + SetStreamValid(false); } @@ -395,6 +415,11 @@ void ScTable::DeleteArea(SCCOL nCol1, SCROW nRow1, SCCOL nCol2, SCROW nRow2, USH ApplyPatternArea( nCol1, nRow1, nCol2, nRow2, aPattern ); } } + + if (IsStreamValid()) + // TODO: In the future we may want to check if the table has been + // really modified before setting the stream invalid. + SetStreamValid(false); } @@ -418,6 +443,11 @@ void ScTable::DeleteSelection( USHORT nDelFlag, const ScMarkData& rMark ) SfxItemPoolCache aCache( pPool, &aSet ); ApplySelectionCache( &aCache, rMark ); } + + if (IsStreamValid()) + // TODO: In the future we may want to check if the table has been + // really modified before setting the stream invalid. + SetStreamValid(false); }
_______________________________________________ LibreOffice mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/libreoffice
