Issue 119954 - [From Symphony]Application crashed if undo/redo covert nest table to text
[From Symphony]Application crashed if undo/redo covert nest table to text
Status: VERIFIED FIXED
Product: Writer
Classification: Application
Component: editing
3.4.0
PC All
: P3 critical (vote)
: 4.0.0
Assigned To: Oliver-Rainer Wittmann
:
Depends on:
Blocks: 120823
  Show dependency treegraph
 
Reported: 2012-06-12 06:39 UTC by Yan Ji
Modified: 2012-10-09 09:36 UTC (History)
5 users (show)

See Also:
Issue Type: DEFECT
Latest Confirmation on: ---
Developer Difficulty: ---


Attachments
Fix for the bug crash occur when undo-redo table with nested tables. (2.16 KB, patch)
2012-06-15 09:34 UTC, kang jian
orw: review+
Details | Diff

Note You need to log in before you can comment on or make changes to this issue.
Description Yan Ji 2012-06-12 06:39:01 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
Comment 1 kang jian 2012-06-15 09:34:09 UTC
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.
Comment 2 Oliver-Rainer Wittmann 2012-06-22 07:34:01 UTC
taking over to review the patch
Comment 3 Oliver-Rainer Wittmann 2012-06-22 09:34:31 UTC
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
Comment 4 Oliver-Rainer Wittmann 2012-06-22 10:02:13 UTC
patch committed into trunk - changed file:
/sw/source/core/edit/edtab.cxx, revision 1352828
Comment 5 Armin Le Grand 2012-06-22 10:16:06 UTC
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) ?
Comment 6 Oliver-Rainer Wittmann 2012-06-22 12:13:30 UTC
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.
Comment 7 Du Jing 2012-08-20 07:56:43 UTC
verified on the AOO3.5
Comment 8 Shenfeng Liu 2012-10-09 09:36:26 UTC
set Target Milestone to AOO 3.5.0 for PM purpose.