Bug 112633 - FILESAVE PPTX: Presentation has some invalid content after RT
Summary: FILESAVE PPTX: Presentation has some invalid content after RT
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: filters and storage (show other bugs)
Version:
(earliest affected)
5.4.0.0.alpha0+
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:6.0.0 target:5.4.4
Keywords: bibisected, bisected, filter:pptx, regression
Depends on:
Blocks: PPTX-Corrupted
  Show dependency treegraph
 
Reported: 2017-09-25 02:38 UTC by Aron Budea
Modified: 2017-11-22 15:42 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
Exported test case after applying a fix (1.58 MB, application/vnd.openxmlformats-officedocument.presentationml.presentation)
2017-10-12 10:18 UTC, Jacobo Aragunde Pérez
Details
Minimal test case (32.89 KB, application/vnd.openxmlformats-officedocument.presentationml.presentation)
2017-10-12 15:15 UTC, Jacobo Aragunde Pérez
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Aron Budea 2017-09-25 02:38:19 UTC
Open the presentation template from the following site:
https://templates.office.com/en-US/Academic-presentation,-pinstripe-and-ribbon-design-(widescreen)-TM03431380
Save it as a PPTX.
Open in PowerPoint.

=> It gives a notification: "PowerPoint couldn't read some content in <filename> and removed it. Please check your presentation if the rest of it looks ok."
The presentation opens afterwards.

Observed with LO 6.0 daily build (2017-09-22_22:54:52, 0db08432c69adc36ed464c1f7c8e672acb3752d) & 5.4.0.3 / Windows 7.
Not reproduced with LO 5.3.0.3.
=> regression

There are a couple of other issues in the roundtripped presentation:
the separator lines are badly off (this is also somewhat of a regression, but a separate issue), and the colors are off as well.
Comment 1 Xisco Faulí 2017-09-25 08:47:18 UTC
Regression introduced by:

author	Tor Lillqvist <tml@collabora.com>	2017-01-24 13:52:28 (GMT)
committer	Tor Lillqvist <tml@collabora.com>	2017-01-24 14:57:45 (GMT)
commit	dda1c7b6fabbda4cf40c21b5fb8cb30f7c1f0743 (patch)
tree	f161f44b2ff6ccae2b4e88a1d6fb21e3431d627d
parent	c0ee5231c9d7a3186fe39e0955320b2bf14f0e50 (diff)
Drop incorrect "artistic" prefix from some token names
Apparently copy/paste mistakes in the
642a252cf1a2f1d08c4bbfcae15527bb82c7664d and
21d4cfe19e2796ebf89c408e292c4473924b2bc4 commits from 2014; most of
the picture effect type tokens added in that commit do have names that
start with "artistic" but not all.

Does not break any unit test. (Does not fix the bug I am investigating
either, though.)

Bisected with: bibisect-linux-64-5.4

Adding Cc: to Tor Lillqvist
Comment 2 Xisco Faulí 2017-09-26 00:13:35 UTC
I tried to revert commit dda1c7b6fabbda4cf40c21b5fb8cb30f7c1f0743 in a local build but I get a corrupted file after RT without the commit..

I also notice that after commit dda1c7b6fabbda4cf40c21b5fb8cb30f7c1f0743, word/media/hdphoto1.wdp is created.

I'm wondering if the issue was always present but not reproducible until dda1c7b6fabbda4cf40c21b5fb8cb30f7c1f0743... I tried to go back before 21d4cfe19e2796ebf89c408e292c4473924b2bc4 but LibreOffice freezes at export time..

Adding Jacobo to the loop
Comment 3 Jacobo Aragunde Pérez 2017-10-11 17:52:42 UTC
Hi Xisco, the wdp file is used to preserve the original image with no effects applied. The original presentation contains such a file at ppt/media/hdphoto1.wdp. Probably the problem is just the wrong path (word/ instead of ppt/).

The bug didn't happen in 5.3 and before the wdp file was not being preserved in those versions due to the problem fixed in dda1c7b.

Let me take this bug, I miss hacking on LO :)
Comment 4 Jacobo Aragunde Pérez 2017-10-12 10:18:29 UTC
Created attachment 136919 [details]
Exported test case after applying a fix

Can somebody please try this file on MS Office? It should be good now, I've applied a patch that should have fixed the issue but I don't have a way to check it with Office right now.
Comment 5 Jacobo Aragunde Pérez 2017-10-12 11:44:18 UTC
Patch sent to gerrit: https://gerrit.libreoffice.org/#/c/43338/
Comment 6 Xisco Faulí 2017-10-12 13:49:07 UTC
(In reply to Jacobo Aragunde Pérez from comment #4)
> Created attachment 136919 [details]
> Exported test case after applying a fix
> 
> Can somebody please try this file on MS Office? It should be good now, I've
> applied a patch that should have fixed the issue but I don't have a way to
> check it with Office right now.

Hi Jacobo,
I still get the error on MSO Office 2010 and slides 9 to 12 are empty
Comment 7 Jacobo Aragunde Pérez 2017-10-12 15:15:21 UTC
Created attachment 136930 [details]
Minimal test case

Contains one small bitmap with an artistic effect applied. RT'ed files produce the same warning about some unreadable contents when opened back. Artistic effect on the RT'ed file cannot be undone.
Comment 8 Jacobo Aragunde Pérez 2017-10-12 15:20:52 UTC
The problem is now the path stored in the rels file:

Original:
<Relationship Id="rId3" Type="http://schemas.microsoft.com/office/2007/relationships/hdphoto" Target="../media/hdphoto1.wdp"/>

Exported:
<Relationship Id="rId2" Type="http://schemas.microsoft.com/office/2007/relationships/hdphoto" Target="media/hdphoto1.wdp"/>
Comment 9 Jacobo Aragunde Pérez 2017-10-12 18:06:07 UTC
I have good and bad news O:-)

On the bright side, I've updated the gerrit patch so it fixes the problem in the minimal test case I uploaded. There is a variety of pptx and xlsx files potentially affected by this problem.

On the dark side, the patch doesn't yet fix the problem with the original test file. There is one difference between the minimal test case and this file, in the latter the image with the effect is part of the slide design. The original presentation links the .wdp file from `ppt/slideLayouts/_rels/slideLayout1.xml.rels` and `ppt/slideLayouts/_rels/slideLayout3.xml.rels` (the two layouts that cointain that image with the green bookmark). The RT'ed file refers to the .wdp file from `ppt/slideMasters/_rels/slideMaster1.xml.rels`. I don't know how this difference can affect the import in MSOffice.

I think the patch is still worth merging because it fixes an existing problem; unfortunately, I cannot commit to keep working to fix the original problem.

For future reference, you can read more about these artistic effects at: https://blogs.igalia.com/jaragunde/2014/06/26/tales-of-libreoffice-interoperability-shape-effects/
Comment 10 Luke 2017-10-13 17:07:54 UTC
Tamás,
Would you be the right one to review this patch?
Comment 11 Commit Notification 2017-10-16 15:19:59 UTC
Jacobo Aragunde Pérez committed a patch related to this issue.
It has been pushed to "master":

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

tdf#112633 oox: Save backup images to proper path in document

It will be available in 6.0.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 12 Commit Notification 2017-10-20 18:07:54 UTC
Jacobo Aragunde Pérez committed a patch related to this issue.
It has been pushed to "libreoffice-5-4":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=3e250d0a392c37e9e610bd83017a3045c4c02638&h=libreoffice-5-4

tdf#112633 oox: Save backup images to proper path in document

It will be available in 5.4.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 13 Xisco Faulí 2017-11-20 15:55:15 UTC
A polite ping to Jacobo Aragunde Pérez: is this bug fixed? if so, could you
please close it as RESOLVED FIXED ? Thanks
Comment 14 Jacobo Aragunde Pérez 2017-11-21 11:11:55 UTC
Hi Xisco, unfortunately the bug is not completely fixed. There were two problems in the original test file, one was fixed but the other, as explained in comment 9, is not.

I don't have time to keep working on this so I'm setting it back to NEW, sorry.