Issue 121008 - [crash]Ctrl+Z twice after merged cells, AOO will crash
[crash]Ctrl+Z twice after merged cells, AOO will crash
Status: VERIFIED FIXED
Product: Calc
Classification: Application
Component: editing
4.0.0-dev
All All
: P3 major (vote)
: 4.0.0
Assigned To: Armin Le Grand
: crash
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-09-18 02:51 UTC by Li Feng Wang
Modified: 2013-07-12 11:00 UTC (History)
5 users (show)

See Also:
Issue Type: DEFECT
Latest Confirmation on: ---
Developer Difficulty: ---
jsc: 4.0.0_release_blocker+


Attachments
Patch for solution (a) (3.36 KB, patch)
2013-06-26 15:42 UTC, Armin Le Grand
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this issue.
Description Li Feng Wang 2012-09-18 02:51:27 UTC
build:
AOO trunk r1384699

steps:
1)new a spreadsheet
2)inset comment to A1 and B3
3)select cells from A1 to B3 and merge these cells
4)press Ctrl+Z twice 

defect:
AOO crash
Comment 1 Andre 2013-06-17 14:16:08 UTC
I can not reproduce this crash on Windows7.
Comment 2 jsc 2013-06-24 12:15:06 UTC
set showstopper flag
Comment 3 Armin Le Grand 2013-06-24 16:13:30 UTC
ALG: Cannot reproduce on Win7
Comment 4 Armin Le Grand 2013-06-24 16:23:34 UTC
ALG: Cannot reproduce on Linux (xubuntu)
Comment 5 Kay 2013-06-24 20:30:00 UTC
This does happen to me with AOO4 on Linux, but, some explanation about ctrl-Z generally for linux might be in order:

http://superuser.com/questions/476873/what-is-effect-of-ctrl-z-on-a-unix-linux-application

The first ctrl-Z has no effect, the second closes AOO4.
Comment 6 Armin Le Grand 2013-06-25 09:37:56 UTC
ALG: Hi Kay, that maybe an explanation. What happens when you do not use the keyboard CTRL_Z, but the undo-buttons (top toolbar)?
Comment 7 Kay 2013-06-25 17:51:26 UTC
Hi Armin --

Basically the same thing. Here's what I did and what happens.

* create a new spreadsheet
* put values an A1, and B3
* add comments to A1 and B3
* select A1 to B3 and merge cells. I respond "yes" to  include hidden cells in merge.
* ctrl-Z once, this undoes the merge. Comments come back also.
* Using ctrl-Z again, or undo (this time on Comment insert as un-merge has already happened), closes AOO 4.


more on this...just now I created a new spreadsheet, input three numbers, did ctrl-Z 3 times to undo the input. Everything is OK.

Now undo/ctrl-Z is greyed out because I can't "undo" anything else.

Doing ctrl-Z again causes no harm, and doesn't close AOO 4. So, my initial thought of ctrl-Z in AOO vs Linux isn't holding up. It has NO effect in simple undo operations.

Something about the merge and un-merge w/comments is causing this odd behavior.
Comment 8 Armin Le Grand 2013-06-26 13:12:12 UTC
ALG: Okay, can reproduce with this description on Win7. Already get asserts on the first actions, taking a look.
Comment 9 Armin Le Grand 2013-06-26 13:15:41 UTC
ALG: Compared with AOO3.4.1, no crash, thus regression. The first undo does the same as in AOO4, the second is different: In AOO3.4.1 it clears and deletes the comment, in AOO4 it removes the edited text from the comment, but not the comment itself. Looks as if the merge/merge undo is not the reason...
Comment 10 Armin Le Grand 2013-06-26 13:19:55 UTC
ALG: Indeed, also happens:
- new Calc
- add comment to cell (any, empty), type text
- CTRL-Z -> comment text is deleted, but comment object not
- 2nd CTRL-Z -> crash
May have to do with the change to include the EditEnginge actions to the current undo stack. Need to check what undos calc is creating.
Comment 11 Armin Le Grand 2013-06-26 14:46:06 UTC
ALG: Reason is that calc does a lot of special handling to get all undo actions for creating and editing a comment (which is a DrawingLayer object in reality) in a single own SfxListUndoAction. This collides with the change that gives the EditEngine an UndoManager from outside to add it's UndoActions there. There are two ways to change this:
(a) Do not use the mechanism to have a single UndoManager and all the TextUndos
(b) Remove all special handling and try if it works
Since Calc's goal it to get all bundled, (a) may be better, but (b) will be more safe for the future and remove those special cases. Looking on possibilities...
Comment 12 Armin Le Grand 2013-06-26 15:42:36 UTC
Created attachment 80918 [details]
Patch for solution (a)

ALG: First try in direction (a), but got a crash after some more actions (moving, resizing, editing, show/hide comment). Will try a (b) solution which promises more stable undo handling without all that calc exceptional handling. May fail for reasons I do not yet see..
Comment 13 Armin Le Grand 2013-06-26 17:04:23 UTC
ALG: (b) is too complicated for now; many other stuff is handled that way from calc, not only adding/changing comments. Concentrating back on (a)...
Comment 14 Armin Le Grand 2013-06-27 09:09:17 UTC
ALG: Betterversion of (a) ready, checking if I can reproduce the other crashes I got from earlier tries; may have been related to debugging and code jumping.
Comment 15 Armin Le Grand 2013-06-27 09:41:03 UTC
ALG: Making the note visible, editing it's text (adding or deleting), ending textedit, UNDO -> crashes. Also crashes without my changes. Seems to have to do with FuText::StopEditMode, checking...
Comment 16 Armin Le Grand 2013-06-27 10:01:24 UTC
ALG: Reason is that for editing text (compared to creating a note and editing text) the DrawingLayer stuff is used, but in FuText::StopEditMode still something is done for comment objects; this is wrong and I changed it. Also saw that Undo/Redo when the comment object has changed its size from editing (deleting a whole line) does not adapt size as needed, looking at the used undo action itself...
Comment 17 Armin Le Grand 2013-06-27 10:20:52 UTC
ALG: Also found. No more crashes or ugly things seen with comments, preparing commit.
Comment 18 Armin Le Grand 2013-06-27 10:25:42 UTC
ALG: Okay, done.
Comment 19 SVN Robot 2013-06-27 10:26:36 UTC
"alg" committed SVN revision 1497286 into trunk:
i121008 corrected calc cell comment stuff
Comment 20 Armin Le Grand 2013-06-27 12:49:24 UTC
ALG: Oops, forgot to grep
Comment 21 Kay 2013-07-08 22:39:11 UTC
YES! FIXED! with rev 1499347
Comment 22 fanyuzhen 2013-07-09 09:58:30 UTC
It's verified fixed in revision 1499347