Apache OpenOffice (AOO) Bugzilla – Issue 77784
After paste "frame" into table with "Formatted Text[RTF]", crash when undo
Last modified: 2013-08-07 14:43:03 UTC
Steps to reproduce : 1.Create a new writer document, "insert"--"frame", and click the "OK" button; 2."Table"--"insert"--"table", and click the "OK" button; 3.select the "Frame" ,and "Ctrl + C"; 4.Locate the cursor in the table, and paste it with "Formatted Text[RTF]"; 5.do the "Undo" operation 2 times, crash.
Reassigned to ES.
Good catch! I sent a crashreport but the ID is not there yet. Search for my SUN e-mail. Summary is i77784.
liuyu->ama: I think the problem is, the table will be deleted while pasting the frame with "Formatted Text[RTF]", and the table node used in SwUndoInsTbl::Undo will be a null pointer. why the table will be deleted while pasting the frame with "Formatted Text[RTF]"?
Yes, the deletion of the table is the first defect which causes the crash later on. This deletion is caused by a wrong assumption in SwRTFParser::SetFlysInDoc(..) Here we have to start analyzing and fixing. Unfortunately there is no expert for this code available, but when we have fixed this issue, we will be the new experts for RTF import of fly frames ;-) I debugged a little bit and obviously the problem starts with the code lines: pNd = pNd-FindTableNode(); if( pNd ) aRg.aStart = *pNd; When I understand this code correctly then works the RTF fly import in the following way: First the content of the fly frame is inserted into the main part of the nodes array. In the next step this content is moved into the special section for fly frames. In our bug case the range (aRg) which is used for this move is expanded because it is inside a table. This is wrong, but if we simply remove this code I think we will not be able to import a fly with a table inside correctly! So we have to find a solution which does not expand the aRg if we want to insert a fly frame into a table but which expands the aRg if we want to import a table inside a fly frame. Resume: SwRTFParser::SetFlysInDoc(..) is the code to debug, understand, analyze and finally fix our bug.
Code freeze for OO2.4 in a few days => new target OOo3.0.
Created attachment 50898 [details] patch file
liuyu->ama: I find a way to solve this problem, please have a look at the patch. As your said, the problem is that the code regarding the range expanding will be executed when the frame with tables inside is moved, but not executed when the frame is moved into a table. So, I think what I should do is to deal with these two situations depending on the table inside or outside. But I found it is not easy to get this message in the SwRTFParser::SetFlysInDoc(), because I could only get the position of the first txtnode(inside the table or not inside) inside the frame. So, I have to save the correct start position from the SwRTFParser::SwRTFParser. What do you think about this? By the way, when paste a frame with a table into a table with "Formatted Text[RTF]", there's still a problem, the table line will be deleted after pasting. I will try to fix that later.
ama->liuyu: Yes, you realized the problem well. We have to detect if the table is part of the fly frame or in the fly frame inserted into a table. Your patch looks for table starting after the insert position. If you find such a table you allow the expansion of the fly content range. Even this works for the bug scenario it will not solve slightly different scenarios. First scenario: Create a table which contains in the first cell a paragraph and another table (table in table). Try to insert our fly frame (RTF-formatted) into this paragraph before the inner table => the problem still occurs because the inner table is detected by your patch. This happens because your loop runs until pNd->EndOfSectionIndex(), why? Wouldn't be pNd->GetIndex() enough? Second scenario: If you do not select only one fly frame, select two paragraphs which two fly frames anchored to them. If you insert this (RTF-formatted) into a table, the problem still occurs. This happens because the first fly frame insertion shifts all nodes in our nodes array. Your saved nSaveCrsrNodeIndex is not correct any longer, it points to a position before the original insert position. When the second fly frame is inserted, it detects the table _behind_ nSaveCrsrNodeIndex and the problem starts again.
ama->liuyu: Instead of starting from the original insertion point we should analyze the nodes before the pNd. pNd->FindTableNode() is wrong because it delivers the table outside the fly frame as well. But if the table is inside the fly frame, it should start immediately before pNd: <table start> <cell start> <pNd> ... In the case above the table is inside and the range has to be expanded. The table below is outside and the range must not to be expanded: <table start> <cell start> <text node> <pNd> ... So you have to look for the node just before pNd, if it is not a <cell start node> there is no need to expand the range. If it is a cell start and the previous node is a table start then the range has to be expanded.
Created attachment 50928 [details] patch file
liuyu->ama: I think I've got the key point. Please have a look at the new patch.
ama->liuyu: Yes, your patch works for inserting fly frames into tables and it works for fly frames containing a table at the beginning. Inserting fly frames with tables inside into a table does not work but this is another issue and it is not caused by SetFlysInDoc(..) I assume. But one thing has to be corrected: if the fly frame contains a table at the end (and at least one paragraph before), it cannot be inserted correctly (RTF formatted) into normal text. It similar to our solved table at the top of fly frame problem, the range has to be adjusted if directly behind the selection is an end node of a cell, followed by a table end node.
Created attachment 51097 [details] patchfile
liuyu->ama: Please have a look at the new patch.
ama->liuyu: Your new patch will cause trouble if you try to insert(RTF-formatted) a fly frame which contains a table at the top, a paragraph behind this table and another table. You should not look for a table inside the fly frame before you check, if aRg.aStart has to manipulated. Just check if the node before aRg.aStart is a table box start node and previous node is the table start node. Then you should change aRg.aStart. The condition if the aRg.aEnd has to be manipulated indeed needs to check if the table is inside the fly frame. But this can be easily done by changing the condition if( !pNd->IsTableNode() && 0 !=(pNd = pNd->FindTableNode()) ) with an additional && pNd->GetIndex() >= aRg.aStart...
Created attachment 51126 [details] patchfile
liuyu->ama: I think I got what you mean. The conditions if the aRg.aStart or the aRg.aEnd to be manipulated are not the same. The aRg.aStart has to be manipulated only when the fly frame start with a table inside and the aRg.aEnd only when the fly frame end with a table inside.
ama->liuyu: Thanks for your work on this issue! Fixed in CWS sw8u10bf05 rtffly.cxx
Ready for QA.
Verified fix in CWS sw8u10bf05.
Checked in build DEV300m24.
*** Issue 92302 has been marked as a duplicate of this issue. ***