Issue 84984 - impress137: crash when using graphic filters on bmp
Summary: impress137: crash when using graphic filters on bmp
Status: CLOSED FIXED
Alias: None
Product: Draw
Classification: Application
Component: ui (show other issues)
Version: 680m237
Hardware: All All
: P3 Trivial (vote)
Target Milestone: OOo 3.0
Assignee: groucho266
QA Contact: issues@graphics
URL:
Keywords:
Depends on:
Blocks: 72764
  Show dependency tree
 
Reported: 2008-01-04 10:10 UTC by wolframgarten
Modified: 2017-05-20 10:27 UTC (History)
4 users (show)

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

Attachments
This patch is based on that of issue 73871. (632 bytes, patch)
2008-02-18 02:00 UTC, liangweike 		no flags Details | Diff
The changes are made once more with the help of Christian. (1.23 KB, patch)
2008-02-27 02:00 UTC, liangweike 		no flags Details | Diff
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this issue.
Description wolframgarten 2008-01-04 10:10:46 UTC 
Open a new draw, insert a bmp and click on one of the graphic filter buttons of
the picture bar: Office crashes. This happens only in CWS impress137 not in
master build.
The ID of the error report is r7vrb9.
Comment 1 liangweike 2008-02-18 02:00:13 UTC 
Created attachment 51553 [details]
This patch is based on that of issue 73871.
Comment 2 pagalmes.lists 2008-02-20 09:42:56 UTC 
shoulnd't that be set as a PATCH issue type?
Comment 3 wolframgarten 2008-02-20 10:28:55 UTC 
No, Ido no thinks so. This is a defect in the named CWS and it should be handled
that way.
Comment 4 liangweike 2008-02-21 01:21:28 UTC 
And I've already committed the patch to CWS impress137. Please check and test it.
Comment 5 wolframgarten 2008-02-21 07:47:57 UTC 
As soon as the CWS gets into status "ready for QA" I will test it...
Comment 6 clippka 2008-02-21 09:48:36 UTC 
whatever the cause of this bug is, the given patch is not valid.

The line

pOL->ReplaceObject(pNewObj,pOldObj->GetOrdNum());

will remove pOldObj from the page and if it is not put into an undo action that
it may also get deleted.

So doing a

MarkObj(pOldObj,&rPV,TRUE);

after the call to ReplaceObject is an error since pOldObj could be deleted
already at this point. Please change the fix.
Comment 7 liangweike 2008-02-22 03:00:14 UTC 
->cl: Thank you for your explanation. However, based on my investigation 
ReplaceObject(...) does not always ensure it will remove the old object because 
it needs to judge a certain condition. Additionally MarkObj(pOldObj,&rPV,TRUE) 
can merely change the mark list to avoid confusion. The above points are just 
simple understanding. So please correct me if I'm wrong.
Comment 8 clippka 2008-02-22 08:28:03 UTC 
->liangweike: after a call to ReplaceObject(), pOldObj will no longer be part of
the model. The only reason that ReplaceObject() will not remove it is if it is
not part of the model or the new object is null. Both cases are fatal errors
already.

In any way, marking pOldObj after it is removed from the model makes no sense
and is an invalid operation. You can not mark objects in a view that are not
part of the model that the view displays.
Comment 9 liangweike 2008-02-27 02:00:28 UTC 
Created attachment 51742 [details]
The changes are made once more with the help of Christian.
Comment 10 groucho266 2008-02-28 17:09:52 UTC 
Fixed by liangweike.
Comment 11 groucho266 2008-04-03 09:45:44 UTC 
Decreasing priority to P3 because the crash is not in the master.
Comment 12 gghh 2008-09-16 17:55:16 UTC 
I've checked in m6. It's ok.
No crashes.