Bug 60910 - FILESAVE loses Fontwork/shape objects
Summary: FILESAVE loses Fontwork/shape objects
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
4.0.0.3 release
Hardware: Other All
: high major
Assignee: Luke Deller
URL:
Whiteboard: BSA target:4.1.0 target:4.0.4 target:...
Keywords: bibisected, bisected, regression
: 60966 62285 62320 63686 64134 65476 67066 70320 (view as bug list)
Depends on:
Blocks: mab4.4
  Show dependency treegraph
 
Reported: 2013-02-15 16:37 UTC by Juan Moreno
Modified: 2015-12-17 07:12 UTC (History)
22 users (show)

See Also:
Crash report or crash signature:


Attachments
oval.doc (27.50 KB, application/msword)
2013-04-24 03:16 UTC, Luke Deller
Details
test for what the fix should not regress (10.66 KB, application/vnd.oasis.opendocument.text)
2015-02-12 22:25 UTC, Lionel Elie Mamane
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Juan Moreno 2013-02-15 16:37:25 UTC
Problem description: When a document with a fontwork is opened the "fontwork" is missing. 

Steps to reproduce:
1. ....Create a new document with a fontwork
2. ....Save the file and close it
3. ....Open the file, the fontwork is missing

Current behavior: Fontwork disappears

Expected behavior: Fontwork doesn't disappear

              
Operating System: Ubuntu
Version: 4.0.0.3 release
Comment 1 Juan Moreno 2013-02-15 17:16:02 UTC
The same problem happens on win7 64 bits and Ubuntu 12.10 64 bits
Comment 2 Juan Moreno 2013-02-15 17:16:36 UTC
(In reply to comment #0)
> Problem description: When a document with a fontwork is opened the
> "fontwork" is missing. 
> 
> Steps to reproduce:
> 1. ....Create a new document with a fontwork
> 2. ....Save the file and close it
> 3. ....Open the file, the fontwork is missing
> 
> Current behavior: Fontwork disappears
> 
> Expected behavior: Fontwork doesn't disappear
> 
>               
> Operating System: Ubuntu
> Version: 4.0.0.3 release
Comment 3 Thomas van der Meulen 2013-02-16 10:23:05 UTC
Thank you for reporthing this bug,
I can reporduce this bug running LibreOffice 4.0.0.3 on Windows 7.
Comment 4 manj_k 2013-02-16 20:32:25 UTC
*** Bug 60966 has been marked as a duplicate of this bug. ***
Comment 5 Jorendc 2013-02-16 20:35:44 UTC
I can confirm this behavior using LibreOffice Version 4.1.0.0.alpha0+ (Build ID: c16e9f4ed97f65357e9986f46ad88ee9f223799) with Linux Mint 14 x64.
Comment 6 Toni Ballesta 2013-03-13 08:59:50 UTC
I confirm that the issue is the same on newer LO 4.0.1.2 release, compiled on Gentoo amd64.
Comment 7 Toni Ballesta 2013-03-13 09:16:07 UTC
Returning to binary package version 3.6.4.3 for Gentoo, and the same document, without problems. The fontwork is saved and restored ok.
Comment 8 Jacques Guilleron 2013-03-13 15:07:04 UTC
*** Bug 62285 has been marked as a duplicate of this bug. ***
Comment 9 christopher.bowhuis 2013-03-14 14:28:34 UTC
Version 4.0.1.2 (Build ID: 400m0(Build:2))

save as .odt and fontwork disappears
save as .docx and fontwork disappears
BUT
save as .doc and fontwork is still there... 
at least for me.
Comment 10 Miklos Vajna 2013-03-22 16:07:46 UTC
Confirmed what is in comment 7, so it's a regression.
Comment 11 Miklos Vajna 2013-03-25 14:25:30 UTC
So it turns out the 4-0 branch-off was still OK; however latest 4-0 doesn't export the shape. Bisect says:

75118d2f9ebdf96250d3a4c78b695085c3527e4e is the first bad commit
commit 75118d2f9ebdf96250d3a4c78b695085c3527e4e
Author: David Tardon <dtardon@redhat.com>
Date:   Tue Dec 18 15:24:28 2012 +0100

    fdo#56267, fdo#56980 propagate shape change to subclasses
    
    It turns out (as witnessed by fdo#56267) that my fix for fdo#56980 only
    cured the symptom, not the cause. The real problem is caused by the
    following sequence of events during ODF import:
    
    1) an SvxCustomShape object is created (XShape iface)
    2) an SdrObjCustomShape object is created for the SvxCustomShape, but it
       is not associated with it (yet)
    3) another SvxCustomShape object is created internally by the
       SdrObjCustomShape and they are associated
    4) an EnhancedCustomShapeEngine is created for this SvxCustomShape by
       SdrObjCustomShape
    5) the SvxCustomShape from point 1 is set to the SdrObjCustomShape
    
    At some point (I did not follow this explicitly) the SvxCustomShape
    cached by the EnhancedCustomShapeEngine loses its (weak) reference to
    the SdrObjCustomShape. This leaves it gutted and all subsequent calls to
    render() return an empty XShape.
    
    The solution is simple: let SdrObjCustomShape know that the associated
    UNO shape has changed, so it can drop the custom shape engine.
    
    Change-Id: I267838ea4857dfcd646f40c811f3ae572237a1e6
    (cherry picked from commit 7fec8dfcaca4efc92516f9af51a3157f1a11ccd7)

David, sorry to hear, it seems we have one more commit in this never-ending game. ;-(
Comment 12 zorba_ 2013-04-09 14:18:11 UTC
In 4.0.2 release this bug is still present.
(Please fix it because for schools and children this function is very important)
Comment 13 mal 2013-04-12 10:33:47 UTC
We are also a school and it's a pretty bad bug for us as well

M
Comment 14 Rainer Bielefeld Retired 2013-04-18 18:02:50 UTC
@all:
May I remind you to keep Summaries up to date? It's a little difference whether the object becomes lost during filesave and fileopen, and queries for DUPs are more promising if the summaries are correct.
Comment 15 Rainer Bielefeld Retired 2013-04-18 18:03:37 UTC
*** Bug 63686 has been marked as a duplicate of this bug. ***
Comment 16 Rainer Bielefeld Retired 2013-04-18 18:06:10 UTC
*** Bug 62320 has been marked as a duplicate of this bug. ***
Comment 17 Luke Deller 2013-04-24 03:16:44 UTC
Created attachment 78406 [details]
oval.doc

This bug also affects .doc files containing shapes

1. Open the attached oval.doc and admire the shape (a blue oval)
2. Save as oval.odt and close it
3. Open oval.odt and the blue oval is missing

This appears to be a symptom of the same bug because it is solved by reverting 75118d2f9ebdf96250d3a4c78b695085c3527e4e.

As per comment 9, saving as .doc is not affected by the issue.  I tested in 4.0.2.2 and git master.
Comment 18 Commit Notification 2013-04-24 15:18:34 UTC
Luke Deller committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=15bbafaaed52182e80f1a24d716a2d181cdc0e17

fix fdo#60910 FILESAVE loses fontwork/shape objects



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 19 Luke Deller 2013-04-27 20:59:54 UTC
This problem occurs because:
- SdrObject::getUnoShape() constructs a UNO shape object using the associated page object (SdrObject::pPage)
- Shapes in Writer use page object of class SwXDrawPage which creats UNO shape objects with the class SwXShape, which implements both XShape and XTextContent interfaces
- In this bug, SdrObject::getUnoShape() is called earlier than SdrObject::SetPage(), so the UNO shape object is created with the class SvxShape which does not implement the XTextContent interface, so it is skipped when outputting the odt text document

I think the proper solution is to invalidate the cached UNO shape object in SdrObject::SetPage

Regarding the line in SdrObjCustomShape::InvalidateRenderGeometry to set mxCustomShapeEngine = 0L:
- added by David in commit 76350361f386b78e1bc9edb75af89e7ff3afe356 which introduced crash bug fdo#58267
- removed by David in commit 75118d2f9ebdf96250d3a4c78b695085c3527e4e which introduced the bug in this item (fdo#60910)
- readded by my own commit for this item
- no longer causes the crash analysed in fdo#58267 due to David's commit 31b93b8600a3e219d33173aa68d9ab570e477e50

This line prevents the present bug because the custom shape engine is the only object holding a strong reference to the UNO shape object, so when it is destroyed the UNO shape object will be destroyed too.

I am going to submit another patch to remove this line again as it invalidates the shape more often than needed, in favour of a change to clear the UNO shape when SdrObject::SetPage is called.
Comment 20 David Tardon 2013-04-28 06:47:10 UTC
(In reply to comment #19)
> This problem occurs because:
> - SdrObject::getUnoShape() constructs a UNO shape object using the
> associated page object (SdrObject::pPage)
> - Shapes in Writer use page object of class SwXDrawPage which creats UNO
> shape objects with the class SwXShape, which implements both XShape and
> XTextContent interfaces
> - In this bug, SdrObject::getUnoShape() is called earlier than
> SdrObject::SetPage(), so the UNO shape object is created with the class
> SvxShape which does not implement the XTextContent interface, so it is
> skipped when outputting the odt text document

Right. That is where I got with my analysis too.

> 
> I think the proper solution is to invalidate the cached UNO shape object in
> SdrObject::SetPage

This indeed sounds like it should work. It can still cause more invalidating than necessary (e.g., when moving an object to another page in the same document, or when copying an object to a different document of the same type), but I do not think that matters much.
Comment 21 Commit Notification 2013-04-28 11:58:38 UTC
Luke Deller committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=57082b1243e86694b72c5e4fad013bf207bfe81a

fdo#60910: discard UNO shape object in SdrObject::SetPage



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 22 Commit Notification 2013-04-29 12:48:19 UTC
Luke Deller committed a patch related to this issue.
It has been pushed to "libreoffice-4-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=879c36b01d3f806937cfc26b90ebac89e6df87c4&h=libreoffice-4-0

fdo#60910: discard UNO shape object in SdrObject::SetPage


It will be available in LibreOffice 4.0.4.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 23 Jorendc 2013-04-29 13:05:05 UTC
I can verify this is fixed in current master version (Mac OSX 10.8.3) LibreOffice Version: 4.1.0.0.alpha0+ Build ID: 1cd9b5d859a6468164b043b0fcaaf49c1907500 (=master~2013-04-29_05.54.57_LibreOfficeDev_4.1.0.0.alpha0_MacOS_x86-64.dmg)

Thanks!
Joren
Comment 24 David Tardon 2013-05-02 07:10:34 UTC
*** Bug 64134 has been marked as a duplicate of this bug. ***
Comment 25 mal 2013-05-03 08:40:44 UTC
Is there any chance of getting this into 4.0.3 ?

Ta

M
Comment 26 Fridrich Strba 2013-05-03 10:13:59 UTC
(In reply to comment #25)
> Is there any chance of getting this into 4.0.3 ?

No.
Comment 27 schorschi.schorsch 2013-06-06 20:28:34 UTC
*** Bug 65476 has been marked as a duplicate of this bug. ***
Comment 28 Mike Kaganski 2013-10-10 08:55:24 UTC
*** Bug 70320 has been marked as a duplicate of this bug. ***
Comment 29 Mike Kaganski 2013-10-10 08:59:55 UTC
*** Bug 67066 has been marked as a duplicate of this bug. ***
Comment 30 pierre-yves samyn 2015-02-08 15:02:46 UTC
Hello

Sorry, but stills occurs on windows 7 & Version: 4.4.0.3
Build ID: de093506bcdc5fafd9023ee680b8c60e3e0645d7
LocaleĀ : fr_FR

Steps to reproduce:
1. ....Create a new document with a fontwork
2. ....Save the file (ODT) and close it
3. ....Open the file, the fontwork is missing

Current behavior: Fontwork disappears

Expected behavior: Fontwork doesn't disappear

Regards
Pierre-Yves
Comment 31 mal 2015-02-10 10:19:18 UTC
Also happens in Linux ( openSuse 13.1 x64 - Factory 4.4.0.3 )
Comment 32 Cor Nouws 2015-02-10 10:32:30 UTC
confirm on 440rc3 Ubuntu 32 bits

and on master Version: 4.5.0.0.alpha0+
Build ID: 39e9e3ee8369eeb00025291e637f0bb2a8f87ed2
TinderBox: Linux-rpm_deb-x86@45-TDF, Branch:master, Time: 2015-02-07_22:46:27
Locale: nl_NL

There seems to be need for a unit test .. ;)
Comment 33 mal 2015-02-11 10:27:07 UTC
Does the version need to be changed so it shows up in the 'all reported bugs'
for this version ? This was fine in 4.3.5 ( and since 4.0.4 )

M
Comment 34 Matthew Francis 2015-02-12 12:51:18 UTC
The most recent breakage of saving fontworks seems to have occurred at the below commit.
(it's in the middle of a build breakage, so tested by cherry-picking onto 1cdb723b27d7359097891c7c370264c9974079db)

Adding Cc: to lionel@mamane.lu; Could you possibly take a look at this? Thanks


commit 0685b2e73e48adb84cd01355d45551ab478ebcd5
Author: Lionel Elie Mamane <lionel@mamane.lu>
Date:   Fri Dec 19 23:25:21 2014 +0100

    Assume that as long as the model is the same, the shape doesn't change.
    
    Even if it is moved from not a page to a page or vice-versa.
    This allows assumptions made in the Base Form wizard to hold,
    namely that if one:
    1) Creates controls (and their associated shapes)
    2) Groups the shapes (in a GroupShape)
    Then all the shapes still remember their associated control
    and vice-versa.
    
    Change-Id: I31975970e7ea2f7978aea7f753de88ecd8e55234
Comment 35 Lionel Elie Mamane 2015-02-12 22:25:49 UTC
Created attachment 113352 [details]
test for what the fix should not regress

David Tardon? This is https://gerrit.libreoffice.org/13559 biting us back. I prepared a test in Basic of the issue that my commit was supposed to fix, here it is.

Since this is your area, could you please take a look at this, and try to see how we can solve this without breaking neither fontworks, nor the attached code?

Or Luke, if you have an idea?

Thanks!
Comment 36 Lionel Elie Mamane 2015-02-12 22:29:13 UTC
Note that the test runs successfully in LibreOffice 3.5.4.2, so breaking it is a regression.
Comment 37 David Tardon 2015-02-13 08:59:59 UTC
dtardon->lionel: Could you create a new bug for the Basic problem? The immediate fix for this one is to revert the problematic commit (like I said in the review).
Comment 38 Commit Notification 2015-02-13 09:02:18 UTC
David Tardon committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=ecf0c4ba89e27cbb8cf7c7dd904cdd0b87dd5f52

tdf#60910 don't lose fontwork objects

It will be available in 4.5.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 39 mal 2015-02-13 09:29:29 UTC
Can this be added to 4.4.1 ?
4.5.0 is a long way away and
for us it's a pretty nasty bug

M
Comment 40 David Tardon 2015-02-13 09:39:16 UTC
You have to have some patience... I cannot just push into released branches--the commit has to go through a review.
Comment 41 Commit Notification 2015-02-13 09:57:11 UTC
David Tardon committed a patch related to this issue.
It has been pushed to "libreoffice-4-4":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=600d6eda0028a7b4927547970a9d683a1437e4c8&h=libreoffice-4-4

tdf#60910 don't lose fontwork objects

It will be available in 4.4.2.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 42 mal 2015-02-13 10:05:14 UTC
Sorry, just a bit enthusiastic.

M
Comment 43 pierre-yves samyn 2015-04-22 13:16:17 UTC
Hi

No more reproduced (WORKSFORME) on windows 7/64 & Version: 5.0.0.0.alpha1+
Build ID: 636c5a63d67b52b0d2f9f21a863c45eca6ac9ff7
TinderBox: Win-x86@42, Branch:master, Time: 2015-04-21_23:00:42
Locale: fr_FR

Thank you
Regards
Pierre-Yves
Comment 44 Robinson Tryon (qubit) 2015-12-17 07:12:03 UTC
Migrating Whiteboard tags to Keywords: (bibisected)
[NinjaEdit]