Issue 97421 - Writer loops after deleting invisible recorded changes and Undo
Summary: Writer loops after deleting invisible recorded changes and Undo
Status: CLOSED FIXED
Alias: None
Product: Writer
Classification: Application
Component: code (show other issues)
Version: DEV300m37
Hardware: All All
: P2 Trivial (vote)
Target Milestone: ---
Assignee: michael.ruess
QA Contact: issues@sw
URL:
Keywords:
Depends on:
Blocks: 84292
  Show dependency tree
 
Reported: 2008-12-19 08:35 UTC by amy2008
Modified: 2013-08-07 14:44 UTC (History)
4 users (show)

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


Attachments
patch file (950 bytes, text/plain)
2009-08-24 09:23 UTC, majun51
no flags Details

Note You need to log in before you can comment on or make changes to this issue.
Description amy2008 2008-12-19 08:35:40 UTC
Can reproduce it in Writer with DEV300m37 on WinXP and Fedora

How to reproduce it 
1 Create a new Writer document, Edit - Changes - check 'Record'
2 Type ENTER 3 or more than 3 times, Edit - Changes - uncheck uncheck 'Record'
  Set cursor among these paragraphs,enter a character what you like, e.g. 'a'.
3 Edit - Changes - check 'Record', delete the character you entered just now
4 Edit - Changes - uncheck 'Show' and uncheck 'Record'
5 CTRL + A, Delete, Undo on the Standard bar

Result 
OOo hangs

Expectation
OOo works well

Regards
Li Meiying
Comment 1 michael.ruess 2008-12-19 16:54:42 UTC
MRU->AMA: follow the described steps and Writer will be caught in a loop after
using Undo in the end.
Comment 2 amy2008 2008-12-22 02:30:02 UTC
Issue 97420 and Issue 97421, they are related to 'UNDO' which contains empty 
entries in the listitem.
Comment 3 andreas.martens 2009-07-08 14:52:46 UTC
ama_>mba: Not sure, who could take care of this..
Comment 4 majun51 2009-08-24 09:23:37 UTC
Created attachment 64332 [details]
patch file
Comment 5 majun51 2009-08-24 09:24:30 UTC
Please have a check,thanks in advance.
Comment 6 majun51 2009-10-30 03:36:12 UTC
hi mba,

Could you please check this patch and point out if anything wrong or missed?
Thanks very much.(It seems solve Issue 97420 too )

In this patch:
- changed a critical case in ComparePosition()
- saved nodes for the hidden redlines 
Comment 7 Mathias_Bauer 2009-11-11 13:56:08 UTC
Thanks for the patch, I will have a look at it soon.
Comment 8 majun51 2009-12-18 07:47:59 UTC
hi mba,

I tested a some bigger document which contains some redlines in  and found it
spend too much time on openning file...  it's because my change in
ComparePosition() in the patch. So please give it up.

i just changed "if( rStt1 < rStt2 )" to "if( rStt1 <= rStt2 )". The
ComparePosition() is called in AppendRedline() and according result returned
from ComparePosition(), many redlines will be inserted into pRedlineTbl. Could
you please give me some suggestions about the logic to handle redlines here? I
will continue working on this issue.
Comment 9 mst.ooo 2010-02-05 16:08:00 UTC
hi majun51,

i'm sorry, but your proposed change to ComparePosition is wrong:
try e.g. 2 positions with same start and end, which should return EQUAL, but
returns OUTSIDE.

It seems the return value of ComparePosition is not very well defined if start
and end of one range are equal; but i would recommend not changing
ComparePosition, because nobody knows what that may break.

While debugging i could see that there is a loop in SwDoc::AppendRedline for
such redlines with no extent.
It loops because the statement on line 923 "pRedl->SetStart" does not change the
redline.
Probably it would be better to just change the specific case in
SwDoc::AppendRedline that loops, by checking in the POS_INSIDE case if one of
the ranges is a point.

This has the advantage that it is unlikely to break any unrelated functionality.
Of course this has the disadvantage that it makes AppendRedline even more
complicated, but i cannot think of a better approach.
Comment 10 Mathias_Bauer 2010-05-10 15:12:54 UTC
please take over
Comment 11 mst.ooo 2010-05-18 16:54:17 UTC
@majun51:

after not hearing from you for some time,
i've tried to fix the issue myself now.

you were right about the second part of your patch.

because on Undo, the RestoreSection method clears the section pointer (pMvStt),
the Redo implementation must re-create the redline data.

this was not clear to me initially, because this problem only occurs on Redo.

the other half of the fix is a change in AppendRedline,
as described in my previous comment.

thanks for your patch!
Comment 12 mst.ooo 2010-05-18 16:56:57 UTC
fixed in cws sw33bf04
http://hg.services.openoffice.org/hg/cws/sw33bf04/rev/f78229069e7b
Comment 13 mst.ooo 2010-05-27 11:05:31 UTC
please verify
Comment 14 michael.ruess 2010-06-02 12:51:26 UTC
Verified in CWS sw33bf04.
Comment 15 michael.ruess 2010-06-18 14:03:28 UTC
Checked in DEV300m83.