Bug 90664 - Impress, Changing style text broke animation
Summary: Impress, Changing style text broke animation
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Impress (show other bugs)
Version:
(earliest affected)
3.4.0 release
Hardware: Other All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard:
Keywords: preBibisect, regression
Depends on:
Blocks: Object-Animations
  Show dependency treegraph
 
Reported: 2015-04-16 19:52 UTC by Pierre C
Modified: 2023-10-30 13:53 UTC (History)
8 users (show)

See Also:
Crash report or crash signature:


Attachments
first step (11.19 KB, application/vnd.oasis.opendocument.presentation)
2015-04-16 19:52 UTC, Pierre C
Details
second step, just chnaging background color (12.66 KB, application/vnd.oasis.opendocument.presentation)
2015-04-16 19:53 UTC, Pierre C
Details
third step, adding shadow (12.79 KB, application/vnd.oasis.opendocument.presentation)
2015-04-16 19:54 UTC, Pierre C
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Pierre C 2015-04-16 19:52:20 UTC
Step to reproduce :

1 Create a new presentation

2 Add some text (F2)

3 Choose "text" style

4 Add a custom animation and choose "Appear"

5 Launch your slide (F5)

--> At click, your text appear

7 --> Change Text Style : Fill background with a colour (tab Area/Fill/Color/tango...)

8 Launch your slide

--> there is a rectangle and the text appears after click. this is not the correct behaviour. Both rectangle and text should appear at the same time

9 Change Text Style : use shadow

10 Launch your slide
--> the text box is visible, and shadow appears at click. All the text box should appears at click
Comment 1 Pierre C 2015-04-16 19:52:58 UTC
Created attachment 114838 [details]
first step
Comment 2 Pierre C 2015-04-16 19:53:44 UTC
Created attachment 114839 [details]
second step, just chnaging background color
Comment 3 Pierre C 2015-04-16 19:54:34 UTC
Created attachment 114840 [details]
third step, adding shadow
Comment 4 Buovjaga 2015-04-18 11:58:13 UTC
Reproduced.

Win 7 Pro 64-bit, Version: 4.4.2.2
Build ID: c4c7d32d0d49397cad38d62472b0bc8acff48dd6
Locale: fi_FI

Version: 5.0.0.0.alpha0+ (x64)
Build ID: 211c12b9c64facd1c12f637a5229bd6a6feb032a
TinderBox: Win-x86_64@42, Branch:master, Time: 2015-04-18_01:51:17
Locale: fi_FI

Ubuntu 14.10 64-bit
Version: 4.2.0.4
Build ID: 05dceb5d363845f2cf968344d7adab8dcfb2ba71
Comment 5 Pierre C 2015-04-19 14:25:56 UTC
Yes, it may be a minor bug, but, consider this :

You're using style because your enjoy Libreoffice's styles. One of your styles is named Important

You use animations in your presentation. And the Important points appears at the end of each slide

And one day, you decide to change your Important style to have better focus on it from peoples 

So, you add background colour, and a shadow effect, to your Important style.

By this way, all your slides are broken.

This is not really a data-loss, but hours of work are lost
Comment 6 Pierre C 2015-04-19 15:25:52 UTC
bug is present in LO 3.6.7, but it works fine with LO 3.3.0.4

So adding keyword regression and changing to version 3.6.7
Comment 7 Buovjaga 2015-04-19 15:32:35 UTC
Thanks for testing. Problem is already in 3.5.0, can't be bibisected.

LibreOffice 3.5.0rc3 
Build ID: 7e68ba2-a744ebf-1f241b7-c506db1-7d53735
Comment 8 Pierre C 2015-04-19 16:16:07 UTC
Problem is already in 3.4.0.1

Is it useful to find between witch version problem appears ?
Comment 9 Buovjaga 2015-04-19 16:26:42 UTC
(In reply to Pierre C from comment #8)
> Problem is already in 3.4.0.1
> 
> Is it useful to find between witch version problem appears ?

Well I guess it couldn't hurt :) Perhaps some impress dev can then locate the problem faster.
Best would be through bibisecting, but we can't do it for versions older than 3.5.0: https://wiki.documentfoundation.org/QA/HowToBibisect

Yet there are only 4 release versions between 3.3.0 and 3.4.0, so it shouldn't take long for you to find the exact release where the problem appeared: http://downloadarchive.documentfoundation.org/libreoffice/old/
Comment 10 Pierre C 2015-10-27 08:46:32 UTC
Problem is NOT in 3.3.4.1 So It starts with LO 3.4

Problem is Also In AOo, So the problem may come from an AOo Commits integrated to LO
Comment 11 Pierre C 2015-10-27 11:40:33 UTC
If confirm this :
It works fine with OOo 3.3 and it does not work with AOo 3.4.1
So the regression may come from an AOo commit.
If all of this can help. this is really an annoying bug, when using styles with Impress
Comment 12 Robinson Tryon (qubit) 2015-12-14 05:40:11 UTC Comment hidden (obsolete)
Comment 13 Jean-Baptiste Faure 2016-04-12 07:31:00 UTC
Reproducible with current master (Version: 5.2.0.0.alpha0+
Build ID: cf1ecad26d22e3dc5f556f976bdc49a31bfa5630) built at home under Ubuntu 15.10 x86-64.
That said if I add a second text box with style Text, then add an animation to it and finally modify the Text style (for example add shadow), then it works as expected for that textbox.

Best regards. JBF
Comment 14 QA Administrators 2017-05-22 13:27:59 UTC Comment hidden (obsolete)
Comment 15 QA Administrators 2019-12-03 14:35:57 UTC Comment hidden (obsolete)
Comment 16 Pierre C 2020-11-12 05:45:20 UTC
Still a bug with LO 7.1 dev
Comment 17 QA Administrators 2022-11-13 03:32:22 UTC Comment hidden (obsolete)
Comment 18 Julien Nabet 2023-09-09 15:19:47 UTC
On pc Debian x86-64 with master sources updated today, 
if I create a textbox with just "Test1" and style, when creating custom animation, the effect will be called "Test1" and below "Entrance: Appear".
I could reproduce this.

Now if I create a textbox with just "Test2" and style + modify style to have a color for background, when creating custom animation, the effect will be called "Text Frame 'Test2' 2:Test2" and below "Entrance: Appear".
I don't reproduce this.

Thanks to the info from comment 10 (thank you Pierre!) + some debug to find which kind of files were used when adding a custom effect, I could spot this patch:
"
commit 5d5325135c31f829fbaedbf9a1403f90474c2772
Author: Christian Lippka <cl@openoffice.org>
Date:   Thu Dec 2 19:57:45 2010 +0100

    impressdefaults1: #i113014# animate by 1st level paragraph for text shapes by default
"
when reverting it, it works.
Precisely, when I create a Textbox with Test1 + select a style + add some custom anim (so case 1), the effect name "Text Frame" and in this case, the slideshow works well.

I put in "See Also" the bug this quoted commit fixed.

Of course, we could just revert the patch but since #i113014# isn't clear for me...

Thorsten/Armin: any thoughts here as Impress Draw experts or do you know whom to ping?
Comment 19 Armin Le Grand (allotropia) 2023-10-23 13:14:49 UTC
Well, Lippka was one of the sun-time dev members, he did a lot of things. Thus the change is really old - maybe it can be read from that change what it tried to fix, just to be careful?
I have no deep idea of that animation stuff added as properties, so I would suggest
- If removal works, fine
- If no test breaks & playing around looks goog, fine
- As mentioned above: Try to derive a purpose of that orig change & eval if it was/is needed and if it may make another situation worse: if not, fine
Comment 20 Julien Nabet 2023-10-24 15:19:32 UTC
Thank you Armin for your feedback.

When creating a shape without customizing style, LO goes to line 1859:
   1853             // if only one shape with text and no fill or outline is selected, animate only by first level paragraphs
   1854             if( bHasText && (aTargets.size() == 1) )
   1855             {
   1856                 Reference< XShape > xShape( rTarget, UNO_QUERY );
   1857                 if( xShape.is() && !hasVisibleShape( xShape ) )
   1858                 {
   1859                     mpMainSequence->createTextGroup( pCreated, 1, -1.0, false, false );
   1860                 }
   1861             }

We depend on hasVisibleShape which contains this:
1059          if( sShapeType == "com.sun.star.presentation.TitleTextShape" || sShapeType == "com.sun.star.presentation.OutlinerShape" ||
1060              sShapeType == "com.sun.star.presentation.SubtitleShape" || sShapeType == "com.sun.star.drawing.TextShape" )
1061          {
1062              Reference< XPropertySet > xSet( xShape, UNO_QUERY_THROW );
1063  
1064              FillStyle eFillStyle;
1065              xSet->getPropertyValue( "FillStyle" ) >>= eFillStyle;
1066  
1067              css::drawing::LineStyle eLineStyle;
1068              xSet->getPropertyValue( "LineStyle" ) >>= eLineStyle;
1069  
1070              return eFillStyle != FillStyle_NONE || eLineStyle != css::drawing::LineStyle_NONE;
1071          }

So when there's no fillstyle and no line style, it returns false and LO calls createTextGroup.

The pb is once createTextGroup is called, if fillstyle or line style changes, we can't revert.

I don't know how to test "animate by 1st level paragraph for text shapes by default" but I suppose reverting would cause some regression.
=> we're stuck here.
Comment 21 Thorsten Behrens (allotropia) 2023-10-30 13:53:44 UTC
Currently, Impress seems to assume that 'having background fill' is equivalent to 'this is not a real text element'.

The comment in the commit that is referenced seems to match what the code is doing:

  // if only one shape with text and no fill or outline is selected, animate only by  first level paragraphs

Whether that is a useful heuristic or not, I have no opinion - but at least the code seems to do as advertized? Perhaps it would be a good next step, to precisely define the behaviours we expect here (and then we can argue which code is needed, and what is buggy).