Issue 119945 - [From Symphony]Application crashed if undo add caption to drawing object
Summary: [From Symphony]Application crashed if undo add caption to drawing object
Status: CLOSED FIXED
Alias: None
Product: Writer
Classification: Application
Component: editing (show other issues)
Version: 3.4.0
Hardware: PC All
: P3 Critical (vote)
Target Milestone: 4.0.0
Assignee: Oliver-Rainer Wittmann
QA Contact:
URL:
Keywords:
Depends on:
Blocks: 120823
  Show dependency tree
 
Reported: 2012-06-12 03:23 UTC by Yan Ji
Modified: 2012-10-09 09:31 UTC (History)
5 users (show)

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


Attachments
patch to fix the issue in i119945 (3.11 KB, patch)
2012-06-14 03:25 UTC, yuanlin
orw: review+
Details | Diff

Note You need to log in before you can comment on or make changes to this issue.
Description Yan Ji 2012-06-12 03:23:38 UTC
Build: AOO 3.4
Steps:
1. New Text document
2. Insert two drawing objects(eg:Rectangle Ellipse).
3. Group the two objects then ungroup them.
4. Select the Rectangle and add a Caption for it.
5. Press Ctrl+Z twice then click the Ellipse
6. Application crash.
Comment 1 yuanlin 2012-06-14 03:20:22 UTC
In SdrUndoRemoveObj, it stores and uses the OrdNum(order number) in SdrObjList to restore the corresponding objects. However, when insert a caption for a drawing object, a new SwFlyFrm will be created and the order of that drawing object in SdrObjList will be adjust to be right after the SwFlyFrm, which contains the caption. Below is an example in sw\source\core\layout\flylay.cxx

void SwPageFrm::AppendFlyToPage( SwFlyFrm *pNew )
{
  ....
  const SwFlyFrm* pFly = pNew->GetAnchorFrm()->FindFlyFrm();
	if ( pFly && pObj->GetOrdNum() < pFly->GetVirtDrawObj()->GetOrdNum() )
	{
        sal_uInt32 nNewNum = pFly->GetVirtDrawObj()->GetOrdNumDirect();
		if ( pObj->GetPage() )
			pObj->GetPage()->SetObjectOrdNum( pObj->GetOrdNumDirect(), nNewNum);
		else
			pObj->SetOrdNum( nNewNum );
	}
	...
}

So when the SdrUndoRemoveObj use the stored OrdNum to restore the objects, strange things will happen such as crash. It can be verified by that if add caption for the last created drawing objects, AOO will not crash. But if add caption for other drawing objects, AOO will crash.

The idea to fix this issue is that when adjust the order for the drawing object and new SwFlyFrm, do not change the original drawing objects's OrdNum, but change the SwFlyFrm's OrdNum which is added later than the drawing object. So when do Undo operation for caption and the drawing object, the OrdNum will be the same as that stored in SdrUndoRemoveObj.

There are 3 places to adjust the order when insert a SwFlyFrm. Below is one fix example
void SwPageFrm::AppendFlyToPage( SwFlyFrm *pNew )
{
  ....

  SwFlyFrm* pFly = (SwFlyFrm*)pNew->GetAnchorFrm()->FindFlyFrm();
	if ( pFly && pObj->GetOrdNum() < pFly->GetVirtDrawObj()->GetOrdNum() )
	{
		//#i119945# set pFly's OrdNum to _rNewObj's. So when pFly is removed by Undo, the original OrdNum will not be changed.
        sal_uInt32 nNewNum = pObj->GetOrdNumDirect();
		if ( pObj->GetPage() )
			pObj->GetPage()->SetObjectOrdNum( pFly->GetVirtDrawObj()->GetOrdNumDirect(), nNewNum );
		else
			pFly->GetVirtDrawObj()->SetOrdNum( nNewNum );
	}
	...
}
Comment 2 yuanlin 2012-06-14 03:25:02 UTC
Created attachment 78310 [details]
patch to fix the issue in i119945
Comment 3 Oliver-Rainer Wittmann 2012-06-21 12:57:30 UTC
taking over to review the patch
Comment 4 Oliver-Rainer Wittmann 2012-06-21 15:17:23 UTC
Comment on attachment 78310 [details]
patch to fix the issue in i119945

The patch looks good. I have checked it on my Windows environment and will integrate the patch into the source code repository.
Comment 5 Oliver-Rainer Wittmann 2012-06-22 07:29:23 UTC
committed patch into trunk - changed files:
/sw/source/core/layout/flylay.cxx, 
/sw/source/core/layout/frmtool.cxx
revision 1352786
Comment 6 Du Jing 2012-08-20 07:34:49 UTC
verified on the build AOO3.5
Comment 7 Shenfeng Liu 2012-10-09 09:31:20 UTC
set Target Milestone to AOO 3.5.0 for PM purpose.