Verified on Aoo_Trunk_20121214.1915 Rev.1413470, repro it, marked as confirmed.

I can not reproduce a crash, but loading hangs.  Each loop triggers the assertion in sw/source/core/docnodes/nodes.cxx:846.  The violation of the asserted condition does not seem to be handled, which may be part of the problem.

ALG: As Andre writes, an endless loop where the condition

	while( aRg.aStart < aRg.aEnd )

is never met; part of the SwNodes::_MoveNodes mechanism.

I am working on it.

Just saw that instead of assigning this bug to me, I set myself as QA contact.  Fixed that.

set showstopper flag

This bug can also be observed in older versions (3.1.0 and 3.3).

I know now a bit better what goes wrong: RTF document content is stored as nodes in a node array.  Some content is organized as "fly frames", blocks of text or parts of a table, that are displayed at a position that differs from their logical position in the document.  While the RTF document is read, adjacent fly frames that also have the same properties, are merged into larger fly frames.  Later the fly frame nodes are moved inside the node array to their final location.

There seem to be too problems with that:

1) The merging of fly frames seems not to be working correctly.  Look at a part of the bug document, nodes 13 to 27.  Five text nodes are each enclosed by one start and one end node:

  13:    start node ends at 15
  14:     text node
  15:    end node starts at 13
  16:    start node ends at 18
  17:     text node
  18:    end node starts at 16
  19:    start node ends at 21
  20:     text node
  21:    end node starts at 19
  22:    start node ends at 24
  23:     text node
  24:    end node starts at 22
  25:    start node ends at 27
  26:     text node
  27:    end node starts at 25

Lets assume that we have two fly frames that reference the single nodes 17 and 20.  When they are merged the result is the node range 17 to 20.  This includes the end node of text node 17 and the start node of text node 20.  But not the start node of 17 nor the end node of 20.

This leads to a followup bug when the merged fly frames are moved to their final position.  In SwRTFParser::SetFlysInDoc() there is some magic that adapts the start and end nodes of fly frames.  While this works for the end, it does not work for the start node.  This results in the pathological situation that ultimately triggers the crash/loop where the merged and adapted fly frames cover one half and one complete table.  Moving the fly frames works from end to start.  Therefore the second and complete table is moved without problem, the first and incomplete table is not moved correctly and leads to the error.

2) The SwNodes::_MoveNodes() method seems to have problems in some cases.  In one experiment fly frames where not merged.  This resulted in more but smaller fly frames.  Moving them to their final location resulted in a completely broken document.  Apparently there are some constellations of start, end, and destination index that are not handled correctly.

Also, the case mentioned above where one half and one complete table are moved the first half table should be handled correctly because the given start node, that points into the middle of the table, is ignored.  Instead the end node is asked for the real start node.  But this move still results in the error described by this bug.

a) This is an old bug, not a regression.

b) It is not easy to fix without first fully understanding the relevant code parts on several layers (RTF reader, node container, everything in between) and then replace it with a sane architecture.


I can prevent the crash, but that would result in an incorrectly read document.  Maybe we can/should abort reading the document?

Created attachment 80935 [details]
Prevent invalid move of document nodes.

One way to prevent the crash/loop when loading the bug doc. In SwNodes::_MoveNodes() it checks inside the main while loop if a table node with all its "child" nodes are inside the original range.
The patch is simple and allows one call to _MoveNodes() to move some nodes even when others fail.
However, when the bug document is loaded then it results in producing only a blank page.

Created attachment 80936 [details]
Prevent invalid move of document nodes.  Version B

This patch either lets _MoveNodes() move all nodes in the given range or leaves the method before any node is moved.
With the bug document this patch results in a page that shows all the information in the document but layout is not correct.

At least for the bug doc, this seems to be preferable over the other patch.

Please note that the two patches above only prevent the crash/loop.  They do not fix the loading of the RTF bug doc.

I plan to remove the showstopper flag because we can't fix this problem completely.

I suggest to integrate the patch to avoid the crash and target this problem for the future

"af" committed SVN revision 1498507 into trunk:
121479: Prevent crash when loading some RTF documents.

Checked in the second patch (version B).  As this only prevents the crash but does not fix the loading, I leave the status as it is.

Reset assigne to the default "issues@openoffice.apache.org".