Issue 118878

Summary: [From Symphony] Writer crash after modify properties of new Frame
Product: Writer Reporter: Yan Ji <yanji.yj>
Component: uiAssignee: mayongl <mayongl>
Status: CLOSED FIXED QA Contact:
Severity: Critical    
Priority: P2 CC: binbjguo, josef.latt, liushenf, louqingle, mayongl,
Version: 3.4.0 Beta (OOo)   
Target Milestone: 4.0.0   
Hardware: PC   
OS: Windows 7   
Issue Type: DEFECT Latest Confirmation in: ---
Developer Difficulty: ---
Description Flags
patch_in_AdjustColumns_to_fix_this_issue none

Description Yan Ji 2012-02-07 02:37:05 UTC
1)create a new Word Processor doc
2)create a new Frame via click "Insert-->Frame", use default properties.
3)drag mouse to extend the frame
4)select the frame and double click the frame
5)chage properties of frame.In Columns tab,set "Columns"=2 , set "Line"= 4.00pt,then click OK button
6)repeat step 4
7)In "Type" tab,set "Width" = "6.08", "Height" = "2.03";In "Columna" tab,set "Columns" = "3", then click OK button

defect: Writer crashed
Comment 1 jolatt 2012-02-07 16:29:13 UTC
Can't confirm with OOo-dev 3.4.0, OOO340m1 (Build:9586) Rev. 1240836

OS: Ubuntu 11.10, 64 bit
Comment 2 Yan Ji 2012-02-08 00:32:40 UTC
Reproduced agains revision 1240872 with Windows/Mac/RedHat
Comment 3 yuanlin 2012-05-23 02:14:29 UTC
The reproduce step can be simpler:
1) create a new Word Processor doc
2) create a new Frame via click "Insert-->Frame", use default properties.
3) chage properties for frame from "Right-click menu->Frame...". In Columns tab,set "Columns"=2 
4) chage properties for frame from "Right-click menu->Frame...". In Columns tab,set "Columns"=3(a value more than previous). In "Type" tab,set "Width" to another value such as "6.08"
5) click OK button, AOO will crash.
Comment 4 yuanlin 2012-05-23 02:31:01 UTC
I found the root cause. 
The issue is caused by that if the user change width value and column value together, In SwFmt::SetFmtAttr(), it will update all the modified value first. Then in SwFlyFrm::Modify(), it will enumerate all the changed items one by one and call SwFlyFrm::_UpdateAttr() to handle the value changed for each item.

In this case, frame size value changed item will be handled first, it will call SwFlyFrm::FrmSizeChg() to handle it. In that function, it will call SwLayoutFrm::AdjustColumns() to re-calculate the column size in the frame. When enumerate the columns in the frame, it use the new column number value set in SwFmt::SetFmtAttr(). However, the real column object in frame is not updated. That work will be done in column number value changed handler in SwFlyFrm::_UpdateAttr() later. So if the new column value is larger than previous, in SwLayoutFrm::AdjustColumns(), it will enumerate a null column object. So crash will happen.

According to my investigation, the crash should happen on all kinds of platforms.
Comment 5 yuanlin 2012-05-23 02:49:30 UTC
Created attachment 77582 [details]

The real root cause of this issue is that it should use the old property value but not the new modified and not processed property value in the property changed handler. 
However, for this special case, I think we can have a simple fix. In this case, we can just add the check for column frame object and only do sth when it is not null. the SwLayoutFrm::AdjustColumns() will be called again when process column number value changed handler in SwLayoutFrm::ChgColumns() and can be set the correct value in it.
Comment 6 mayongl 2012-06-13 09:03:20 UTC
Patch reviewed. 

There could be other thoughtful refactory do cover some other similar potential issues. 

But this one is neat and safe.
Comment 7 mayongl 2012-06-16 08:07:30 UTC
Fix submitted in r1350879.
Comment 8 louqle 2012-06-18 05:33:12 UTC
verified in r1350879 on Windows 7 and Mac
Comment 9 binguo 2012-06-18 07:07:17 UTC
Verified it on Aoo_Trunk_20120616.1800.1350879 and it does not reproduce, so close it as fixed.
Comment 10 Shenfeng Liu 2012-10-09 09:21:34 UTC
set Target Milestone to AOO 3.5.0 for PM purpose.