Bug 93124 - PPT import: slide titles are misplaced
Summary: PPT import: slide titles are misplaced
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Impress (show other bugs)
Version:
(earliest affected)
4.1 all versions
Hardware: Other All
: medium normal
Assignee: Mike Kaganski
URL:
Whiteboard: target:5.2.0 target:5.1.4 target:5.3....
Keywords: bibisected, bisected, regression
Depends on:
Blocks:
 
Reported: 2015-08-04 19:33 UTC by Andras Timar
Modified: 2017-01-03 14:02 UTC (History)
6 users (show)

See Also:
Crash report or crash signature:


Attachments
bugdoc (PPT) (227.00 KB, application/vnd.ms-powerpoint)
2015-08-04 19:33 UTC, Andras Timar
Details
Test file used to check patch for regressions (3.14 MB, application/vnd.ms-powerpoint)
2016-02-25 10:46 UTC, Mike Kaganski
Details
Improved test file with reference screenshots from MS PowerPoint (4.20 MB, application/vnd.ms-powerpoint)
2016-03-04 10:08 UTC, Mike Kaganski
Details
autofit test document (97.00 KB, application/vnd.ms-powerpoint)
2016-03-20 00:16 UTC, Thorsten Behrens (CIB)
Details
Two PDFs made from attachment 123726, with LO 5.1.1.3 and with LODev with patch applied (40.92 KB, application/x-zip-compressed)
2016-03-20 00:43 UTC, Mike Kaganski
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andras Timar 2015-08-04 19:33:14 UTC
Created attachment 117654 [details]
bugdoc (PPT)

Titles are not at the top, but moved to the middle of the page. This is a regression from commit acccf7a13fd0f87e5aecdc7d5412726a76dba275
Author: Armin Le Grand <alg@apache.org>
Date:   Fri Oct 5 13:02:31 2012 +0000

    Resolves: #i119885# corrected AutoGrowHeight setting...
    
    when importing PPT text boxes
    
    (cherry picked from commit 887c740e39ec1d00366228aae49ef62087a6feb2)
Comment 1 Mike Kaganski 2015-08-29 14:39:11 UTC
Saving copy of this file as pptx using MS PowerPoint and opening it with LO, everything shows right.

The difference is that in pptx version, the "Full width" checkbox is set, and naturally, the text anchor is set to middle-bottom; while in ppt version, the text anchor point is in the left-bottom corner (select title border, then menu "Format->Text", first tab).

Setting the flag makes everything right. By the way, the same applies to Bug 41245 and AOO#119885.

So, in essence, it seems that ppt version fails to set the "Full width" flag.
Comment 2 Mike Kaganski 2015-09-27 13:48:44 UTC
Submitted patch to gerrit: https://gerrit.libreoffice.org/18895
Comment 3 Robinson Tryon (qubit) 2015-12-10 01:18:42 UTC Comment hidden (obsolete)
Comment 4 A (Andy) 2016-01-23 08:45:00 UTC
Reproducible with LO 5.1.0.2, Win 8.1
It is fine in MSO.
Comment 5 Mike Kaganski 2016-02-25 10:46:40 UTC
Created attachment 122983 [details]
Test file used to check patch for regressions

This PPT (created with MS PowerPoint 2013) contains objects (simple ellipses and clouds) with differently aligned text.
Comment 6 Mike Kaganski 2016-03-04 10:08:10 UTC
Created attachment 123223 [details]
Improved test file with reference screenshots from MS PowerPoint

Here the colors are changed to be more readable; each conbination is explained.
Comment 7 Commit Notification 2016-03-19 11:53:29 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "master":

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

tdf#93124: Fix incorrect text fit in imported PPT

It will be available in 5.2.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 8 Thorsten Behrens (CIB) 2016-03-20 00:16:40 UTC
Created attachment 123726 [details]
autofit test document

Sadly, this broke the autofit feature. A more minimal fix would be better.
Comment 9 Commit Notification 2016-03-20 00:19:05 UTC
Thorsten Behrens committed a patch related to this issue.
It has been pushed to "master":

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

Revert "tdf#93124: Fix incorrect text fit in imported PPT"

It will be available in 5.2.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 10 Mike Kaganski 2016-03-20 00:32:00 UTC
(In reply to Thorsten Behrens (CIB) from comment #8)
> Created attachment 123726 [details]
> autofit test document
> 
> Sadly, this broke the autofit feature. A more minimal fix would be better.

Could you please describe what's wrong with the autofit?
I don't see differences with the patch comparative to vanilla 5.1.1.3...
Comment 11 Mike Kaganski 2016-03-20 00:43:18 UTC
Created attachment 123727 [details]
Two PDFs made from attachment 123726 [details], with LO 5.1.1.3 and with LODev with patch applied

Maybe there's a problem with autofit test file?
Comment 12 Thorsten Behrens (CIB) 2016-03-24 10:08:51 UTC
(In reply to Mike Kaganski from comment #10)
> Could you please describe what's wrong with the autofit?
> I don't see differences with the patch comparative to vanilla 5.1.1.3...
>
Oh - sorry, that might not have been obvious indeed. Type some text in
the box containing 'This is a text', until the space is full. Without
the patch, the text auto-sizes. With the patch applied, it simply
overflows the slide downwards.
Comment 13 Andras Timar 2016-03-25 16:16:55 UTC
(In reply to Thorsten Behrens (CIB) from comment #12)
> (In reply to Mike Kaganski from comment #10)
> > Could you please describe what's wrong with the autofit?
> > I don't see differences with the patch comparative to vanilla 5.1.1.3...
> >
> Oh - sorry, that might not have been obvious indeed. Type some text in
> the box containing 'This is a text', until the space is full. Without
> the patch, the text auto-sizes. With the patch applied, it simply
> overflows the slide downwards.

I opened autofit.ppt (attachment 123727 [details]) in PowerPoint 2010. The text box did not have the autofit property. I typed some text in the box until the space was full. And it overflowed the text box. This is not a good bug document (if we accept PowerPoint 2010 as the reference renderer).
Comment 14 Thorsten Behrens (CIB) 2016-03-25 18:55:08 UTC
(In reply to Andras Timar from comment #13)
> I opened autofit.ppt (attachment 123727 [details]) in PowerPoint 2010. The
> text box did not have the autofit property. I typed some text in the box
> until the space was full. And it overflowed the text box. This is not a good
> bug document (if we accept PowerPoint 2010 as the reference renderer).
>
Maybe, sure - though I'm pretty sure the test document is from an
older PPT version.

But that's somewhat beside the point - looking at

https://gerrit.libreoffice.org/#/c/18895/7/filter/source/msfilter/svdfppt.cxx

, is there _any_ input that would enable autofit in that filter?

The patch is pretty intrusive, why not simply fix the bug at hand?
Comment 15 Michael Meeks 2016-03-25 21:21:21 UTC
> is there _any_ input that would enable autofit in that filter?

I believe Mike K is working on a unit test for auto-fit import with two text boxes one fitting, one not etc. - and then we'll avoid this question in future.

This particular chunk of code seems to go in and out like a yoyo across several commits.

We have increasingly competent tests, and test-cases here; I would prefer to move forward in the direction of ever more tests. If we find subsequent problems - we can add more fixes, with more tests =)
Comment 16 Commit Notification 2016-05-05 06:45:22 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "master":

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

tdf#93124: Fix incorrect text fit in imported PPT - take two

It will be available in 5.2.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 17 Commit Notification 2016-05-06 09:05:21 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "libreoffice-5-1":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=ac2732bdc63fa687909fdf03e2a91081003f109e&h=libreoffice-5-1

tdf#93124: Fix incorrect text fit in imported PPT - take two

It will be available in 5.1.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 18 Commit Notification 2016-06-22 12:36:59 UTC
Tor Lillqvist committed a patch related to this issue.
It has been pushed to "master":

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

tdf#93124: The arbitrary number 100 seems to be too high, let's try 50

It will be available in 5.3.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 19 Xisco Faulí 2016-09-15 20:40:05 UTC
Hi,
Is this bug fixed?
If so, could you please close it as RESOLVED FIXED?
Regards
Comment 20 Commit Notification 2017-01-03 14:02:46 UTC
Tor Lillqvist committed a patch related to this issue.
It has been pushed to "libreoffice-5-2":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=8fdb960a4fca24ae4ef3e9706218ca00cb7f4fd0&h=libreoffice-5-2

tdf#93124: The arbitrary number 100 seems to be too high, let's try 50

It will be available in 5.2.5.

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.