Issue 119945

Summary:

[From Symphony]Application crashed if undo add caption to drawing object

Product:

Writer

Reporter:

Yan Ji <yanji.yj>

Component:

editing

Assignee:

Oliver-Rainer Wittmann <orw>

Status:

CLOSED FIXED

QA Contact:

Severity:

Critical

Priority:

CC:

bjdujing, jingbaibetsy, liushenf, orw, yuanlin.ibm

Version:

3.4.0

Target Milestone:

4.0.0

Hardware:

OS:

All

Issue Type:

DEFECT

Latest Confirmation in:

---

Developer Difficulty:

---

Issue Depends on:

Issue Blocks:

120823

Attachments:

Description	Flags
patch to fix the issue in i119945	orw: review+

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.