Issue 93186 - Impress slide not saved correctly
Summary: Impress slide not saved correctly
Status: CLOSED FIXED
Alias: None
Product: Impress
Classification: Application
Component: save-export (show other issues)
Version: OOO300m3
Hardware: All All
: P3 Trivial (vote)
Target Milestone: OOo 3.0
Assignee: wolframgarten
QA Contact: issues@graphics
URL:
Keywords:
Depends on:
Blocks: 88888
  Show dependency tree
 
Reported: 2008-08-27 15:47 UTC by groucho266
Modified: 2008-09-09 18:30 UTC (History)
1 user (show)

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


Attachments
Copy slide and slides look identical: OK. Save, close, reload: slides differ: not OK. (14.17 KB, application/vnd.oasis.opendocument.presentation)
2008-08-27 15:50 UTC, groucho266
no flags Details

Note You need to log in before you can comment on or make changes to this issue.
Description groucho266 2008-08-27 15:47:39 UTC
1. Load attached Impress document.
2. Copy slide by Ctrl-C then Ctrl-V in slide sorter bar.

 Copied slide looks exactly like the original.  As they should.

3. Save, close, load again.

 Bug: Now the two slides look differently.
Comment 1 groucho266 2008-08-27 15:50:12 UTC
Created attachment 56054 [details]
Copy slide and slides look identical: OK.  Save, close, reload: slides differ: not OK.
Comment 2 groucho266 2008-08-27 15:58:58 UTC
The content.xml in the new document created in step 3 has a seemingly broken tag
for the second slide.  The <draw:page> tag of the second slide does not have any
attributes, unlike the tag of the first slide.
Comment 3 groucho266 2008-08-27 17:14:54 UTC
The root cause of this issue is that for the second, copied slide the wrong UNO
API wrapper is created.  Here is how this happens but beware, this is not for
the faint of heart.

The copy constructor of the SdrPage class contains the line

    *this = rSrcPage;

which calls the assignment operator at a time when the new object is not yet
fully initialized.  Especially the virtual function table is not yet properly
set up because the constructor of the derived class (SdPage) is yet to be
called.  Ugly as this is, it gets really bad when the assignment operator
eventually triggers a call to createUnoPage().

Because the SdPage constructor has not yet been executed and the vtable is not
yet in its final state, the virtual createUnoPage() method is called at the
wrong class.  Instead of SdPage it is called at SdrPage.  As a result an
instance of SvxFmDrawPage is created, not SdDrawPage().

This wrong wrapper is then used when the document is saved.  Many Impress
specific properties are not saved and when the document is loaded again the page
with the wrong UNO wrapper looks different.

The line 
 *this = rSrcPage;
is present since 2004 and one might think that this should have lead to problems
earlier.  Well, before a recent change the UNO wrappers have been volatile
objects that were disposed when not used.  Therefore the wrappers have been
created with the wrong class all along but did not live long enough to cause any
real harm.  Only since a recent change the wrappers are kept alive as long as
the SdrPage/SdPage objects.  So now the wrong wrappers are not disposed and
cause this and maybe other problems.
Comment 4 groucho266 2008-08-27 17:37:29 UTC
A fix has to get rid of
   *this = rSrcPage;
in the copy constructor of SdrPage.  Maybe the copy constructor can be replaced
by a Clone() method.  Maybe the assignment operator has to be called in a save way.
Comment 5 wolframgarten 2008-08-28 08:36:26 UTC
Seems to be broken between BEA300m2 and BEB300m3.
Comment 6 wolframgarten 2008-08-28 11:18:05 UTC
Broken between DEV300_m22 and DEV300_m23.
Comment 7 wolframgarten 2008-08-28 12:20:34 UTC
Target set to 3.0 because of Issue being a showstopper...
Comment 8 groucho266 2008-08-28 14:10:18 UTC
Fixed by disposing an API wrapper that may be created during construction of a
new SdrPage object.  This will force the next call to getUnoPage() to create a
new wrapper, then with a fully constructed vtable in place.  When no wrapper is
created during construction then the behavior of the new SdrPage object will not
change.

I decided not to fix the root cause because that needs more time when done
properly than we have right now.  I will open a separate issue for this.

Modified file is
/svx/source/svdraw/svdpage.cxx    rev. 1.65.34.1
Comment 9 groucho266 2008-08-28 14:52:00 UTC
I have opened issue 93212 to tackle the root cause of this issue.
Comment 10 clippka 2008-08-28 16:27:29 UTC
I reviewed the fix and I can confirm that it is a small change that only
reproduces the behavior prior to my changes that triggered the buggy code but
only in the case of the current issue.
Comment 11 wolframgarten 2008-08-29 14:19:23 UTC
Verified in CWS under Linux.
Comment 12 groucho266 2008-08-29 14:20:53 UTC
Please verify.
Comment 13 wolframgarten 2008-08-29 14:27:16 UTC
Verified under Windows, too.
Comment 14 andreschnabel 2008-09-09 18:30:25 UTC
ok in OOo 3.00RC1 -> close