Created attachment 32169 [details]
test document

MRU->HBRINKM: new document, insert some page breaks, insert a frame, Undo ->
cursor jumps to first position of document

very similar to issue 18014

The bug is not completely fixed. 

If the frame in Writer (OOo 2.0.1 RC3 englisch version under Win2K) is selected,
the cursor jumps to the beginning of the first page:

"Insert, Frame, OK", then imediately click on the undo-button: jump to first page. 

"Insert, Frame, OK", click in the text (deselecting the frame), then click on
the undo-button, cursor stays where he shouldt to stay. 

Successfully reproduced the bug on OOo 2.0.1 with WinXP SP2.
Reproducing the bug as follow:

1. Create a new document in Writer
2. Insert page break by pressing Ctrl-Enter (the cursor should now staying in
the beginning position of the 2nd page)
3. Go to "Insert" -> "Frame" ; then press "Ok"
4. Without clicking anywhere else, press "Undo"

Note that the cursor will go to the beginning position of the document (first page).

Follow-up Testing:
1. 1. Create a new document in Writer
2. Insert page break by pressing Ctrl-Enter (the cursor should now staying in
the beginning position of the 2nd page)
3. Type "HellowWorld" and move the cursor to between "wW"
4. Go to "Insert" -> "Frame" ; then press "Ok"
5. Without clicking anywhere else, press "Undo"

The cursor will again move to the beginning position of the document.  
The curosr will move to the beginning position of the document as long as you
immediately press "Undo" follows by inserting a frame, no matter where the
cursor at.

changed target to 2.x

responsibility for undo changed to ama

*** Issue 70860 has been marked as a duplicate of this issue. ***

ama->liuyu: Maybe the following findings are helpful...

Problem:
1. Create new document
2. Insert some text
3. Put the cursor to the end of the text.
4. Insert a text frame.
5. Undo
=> The cursor jumps to the top of the document.

Reason:
When Undo is performed, the current cursor is placed in the text frame. This
text frame vanishes during the Undo action, so the cursor position has to be
readjusted.
Current code:
SwUndoInsLayFmt::Undo(..) calls
SwUndo::RemoveIdxFromSection(..) calls
SwDoc::CorrAbs(..), this methods sets the current cursor from the vanishing node
to the first SwEndNode of the document's nodes array, but this is not a valid
cursor position.
Later on 
SwCrsrShell::EndAction(..) calls
SwCrsrShell::UpdateCrsr(..) calls
SwCrsrShell::ClearUpCrsr(..), this method recognizes the invalid cursor position
and sets it to the top of the document.

Created attachment 46814 [details]
the patch file

liuyu->ama:
I have submitted the patch file, please have a look at it.

Yes, your patch solves this issue for the most use cases and could be integrated
without changes. But I've some ideas how you could make it even better.

1. I said "most" use cases. But if you create more than one view to your
document (Window/New_window) your patch will work for one view only. This is a
little nitpicking, because this is not very usual and actually your patch
improves the situation even for two views.
2. One important goal for me is to make our code more maintainable. So I'm
trying to reduce the dependencies bewtween different parts of our code. From
that point of view I would prefer not to use (and the new include of) the
SwCrsrShell in undo code.

How can we stay with your idea (remember the cursor position at the undo object
and use this if needed) and solve 1. and 2.?
What about the use of a node index (or a pair node index/content index) instead
of the document coordinates? In SwDoc::_MakeFlySelection(..) there is a
parameter rAnchPos which can be used. You could remember the node index of this
parameter in undo (only the index, not the node itself). This index can become a
new optional parameter for SwUndo::RemoveIdxFromSection(..) and can be used to
construct the aPos object. This should work for all views.
What do you think?

.

move to 3.x accroding http://wiki.services.openoffice.org/wiki/Target_3x

We have to change SwUndo::RemoveIdxFromSection(..) somehow. At the moment this
function creates a position aPos at aNodes.GetEndOfPostIts(). This is not a
rational position for any cursor. In the following all cursors (from every
view/shell) will be set to this "position" if they are in the deleted area (e.g.
inside our removed fly frame). Later on this position will be corrected to the
first text position in the document. If SwUndo::RemoveIdxFromSection() would
deliver a more sensible position aPos, no cursor would jump to the start of
document. But which position should be used from SwUndo::RemoveIdxFromSection()
instead of "GetEndOfPostIts"? There are a lot of opportunities:
SwUndo::RemoveIdxFromSection() could be get another parameter with a position or
it could call a new virtual method which is overwritten by SwUndoInsLayFmt.

Created attachment 48547 [details]
Patch

ama->liuyu: I've just attached a patch which sets the cursor to the end of the
document. This is of course not the solution but it shows that changing of
SwUndo::RemoveIdxFromSection(..) is a possible way to go.
At the moment the method is *not* virtual. If you are planning to overload this
function don't forget to make it virtual in the base class as well.
Hope that helps.

Created attachment 48831 [details]
patchfile

liuyu->ama:
I have submitted the patch file, please have a look at it.

I had a look at the patch and I have some questions about it. Let us discuss
this via IRC, I'll send you an email.. 

Created attachment 49138 [details]
patchfile

Liuyu->ama: I have submitted the patch file. I changed the code according to 
the result we discussed last time. Recently I was busy with other tasks, so 
this patch is somewhat late.

ama->liuyu: I have two remarks to your patch.
1. For the construction of the SwUndoInsLayFmt you get the new needed ULONGs
from two different members?! The SwNodeIndex-ULONG you get from rAnchPos but the
SwIndex-ULONG you get from GetEditShell->GetCrsr().. You better use rAnchPos for
the SwIndex-ULONG as well. Even at that moment
GetEditShell()->GetCrsr()->GetPoint() seems to be identical to rAnchPos, it
makes the code more at least more readable.
2. When you create a SwPosition again from those two ULONGs, you need a SwNode
and a SwIndex again. How you get the SwNode is oikay, but you should not create
a SwIndexReg on the heap and use this for SwIndex. (BTW: this is a memory leak
because this object will never be destroyed). If a SwPosition is created with a
SwNode and a SwIndex, there is a "must": if the SwNode is a SwTxtNode, the
SwIndex has to be registered in this SwTxtNode. (Remember: a SwTxtNode is a
SwIndexReg). 
Please have look at my attachment (id=48547) from Sep 27 and at our Writer Wiki
http://wiki.services.openoffice.org/wiki/Writer_Core_And_Layout#Indexes_and_Positions

Created attachment 49375 [details]
patch file

liuyu->ama: I have read the wiki about the Indexes and Positions. Thank you 
very much, it is very helpful.

Created attachment 49468 [details]
Patched patch ;-)

ama->liuyu: Even I have some remarks to your patch (please have a look into my
last attachment), this issue can be regarded as solved!
I will do some minor changes and check your patch in into CWS sw8u10bf01 for
target OOo2.4. Thank you for your work!

Fixed in CWS sw8u10bf01
undobj.hxx
doclay.cxx
undobj1.cxx
shellio.cxx
unins.cxx
untblk.cxx

Liuyu->ama: I have read your remarks in the last attachment, I think your 
changes are exactly better. Thank you very much for your guidance.
I think I could start with an other issue.

Ready for QA.

Verified fix in CWS sw8u10bf01.

Checked fix in OO 2.4 dev build 680m241.