Apache OpenOffice (AOO) Bugzilla – Issue 119954
[From Symphony]Application crashed if undo/redo covert nest table to text
Last modified: 2017-05-20 10:32:06 UTC
Build: AOO3.4 Steps: 1. New a text document 2. Insert a table and then in one cell create another table 3. Select the first table 4. From menu "Table->Covert->Table to Text 5. Undo/Redo Defect: Application crashed
Created attachment 78339 [details] Fix for the bug crash occur when undo-redo table with nested tables. Root cause: 1.After tabletotext was called in SwUndoTblToTxt::Redo(...), node in document was ruined.When the ruined nodes are accessed later, crash occur. 2.Nested tables not be converted to text actually after calling tabletotext function. Solution: 1.Convert nested tables to text also. UT: Case 1: 1.Test that convert single table to text and undo then redo. Case 2: 1.Test that convert nested table to text and undo then redo.
taking over to review the patch
Comment on attachment 78339 [details] Fix for the bug crash occur when undo-redo table with nested tables. review done. patch works fine and solves the problem. I will commit the patch into the source code repository
patch committed into trunk - changed file: /sw/source/core/edit/edtab.cxx, revision 1352828
ALG: A undo for this is already created without this patch, Undo()/Redo() add sub-actions to it as needed. Why is ConvertTableToText/ConvertNestedTablesToText needed? The original version already converts the whole table, including sub-tables. Would it be sufficient to add StartUndo()/EndUndo() encapsulation to where TableToText was called originally (in SwEditShell::TableToText) ?
Due to Armin's concerns I had another deeper look at this issue: The <SwDoc::TableToText(..)> method does not care about nested tables. In the view it looks like as if the nested table has been also converted, but the document model still contains the nested table. To verify this you can just save the document after the conversion TableToText and reload. You will see that the nested table will show up. Without any further deeper dive into the code this is a really inconsistent situation. Thus, I think the idea of the patch to convert all nested tables in a recursive function is a reasonable approach to solve this problem. As each <SwDoc::TableToText(..)> creates its own Undo as a StartUndo/EndUndo encapsulation is needed.
verified on the AOO3.5
set Target Milestone to AOO 3.5.0 for PM purpose.