Re: [Libreoffice] [PATCH] [PUSHED:master] Fix for Bug 37484 - On any animation change, current position in list is lost

2011-06-27 Thread Radek Doulík
Hi Luke,

thanks for the fix! Pushed.

Cheers
Radek

On Sun, 2011-06-26 at 22:23 +1200, Luke Symes wrote:
 Hi Radek,
 
 On Sat, Jun 25, 2011 at 3:47 AM, Radek Doulik r...@novell.com wrote:
 I run into a crash on startup in that part. I quickly fixed
 it, but it
 might be better if you check that it plays nicely with your
 new code. Or
 maybe make the ScrollToAbsPos not crash when called with
 unexpected
 values, -1 for example.
 
 
 Thanks for fixing this! You're right, I didn't test loading a blank
 presentation... Your quick fix fits well with the rest of the code;
 nFirstVis will never be -1 if there are entries in the
 CustomAnimationList. I was sad that ScrollToAbsPos failed; it turns
 out that it's due to there being no entries in the list (I had added a
 check for negative scroll position values which would try to go to
 scroll to the first entry). I have attached a patch for ScrollToAbsPos
 that will check if there are entries in the list first, and if not
 just return. Could you please review/commit this?
 
 
 Kind regards,
 
 
 Luke.

-- 
Radek Doulík r...@novell.com
Novell, Inc.

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] [PATCH] [PUSHED:master] Fix for Bug 37484 - On any animation change, current position in list is lost

2011-06-26 Thread Luke Symes
Hi Radek,

On Sat, Jun 25, 2011 at 3:47 AM, Radek Doulik r...@novell.com wrote:

 I run into a crash on startup in that part. I quickly fixed it, but it
 might be better if you check that it plays nicely with your new code. Or
 maybe make the ScrollToAbsPos not crash when called with unexpected
 values, -1 for example.


Thanks for fixing this! You're right, I didn't test loading a blank
presentation... Your quick fix fits well with the rest of the code;
nFirstVis will never be -1 if there are entries in the CustomAnimationList.
I was sad that ScrollToAbsPos failed; it turns out that it's due to there
being no entries in the list (I had added a check for negative scroll
position values which would try to go to scroll to the first entry). I have
attached a patch for ScrollToAbsPos that will check if there are entries in
the list first, and if not just return. Could you please review/commit this?

Kind regards,

Luke.
From f1b9f0219708da8332f1ba728fce0c63cbec6f4b Mon Sep 17 00:00:00 2001
From: Luke Symes allsy...@gmail.com
Date: Sun, 26 Jun 2011 22:13:01 +1200
Subject: [PATCH] Fix ScrollToAbsPos to work when we have an empty list.

---
 svtools/source/contnr/svimpbox.cxx |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/svtools/source/contnr/svimpbox.cxx b/svtools/source/contnr/svimpbox.cxx
index fd5a649..e0e9efc 100644
--- a/svtools/source/contnr/svimpbox.cxx
+++ b/svtools/source/contnr/svimpbox.cxx
@@ -1014,6 +1014,8 @@ void SvImpLBox::MakeVisible( SvLBoxEntry* pEntry, sal_Bool bMoveToTop )
 
 void SvImpLBox::ScrollToAbsPos( long nPos )
 {
+if( pView-GetVisibleCount() == 0 )
+return;
 long nLastEntryPos = pView-GetAbsPos( pView-Last() );
 
 if( nPos  0 )
-- 
1.7.5.4

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] [PATCH] [PUSHED:master] Fix for Bug 37484 - On any animation change, current position in list is lost

2011-06-24 Thread Radek Doulik
On Wed, 2011-06-22 at 05:03 -0600, Tor Lillqvist wrote:
 Thanks. If these work fine on Linux (i.e. X11), they can't be awfully
 broken on other windowing systems either, I hope. Pushed to master.

I run into a crash on startup in that part. I quickly fixed it, but it
might be better if you check that it plays nicely with your new code. Or
maybe make the ScrollToAbsPos not crash when called with unexpected
values, -1 for example.

It was happening when starting impress without loading presentation. I
guess you checked it only while loading presentation containing custom
animations?

Cheers
Radek

the quick fix:

commit 4be38046a5c2576b5b97ac47f7c0ea17444524ea
Author: Radek Doulik r...@novell.com
Date:   Fri Jun 24 17:10:22 2011 +0200

quick fix to avoid crash on impress's start

diff --git a/sd/source/ui/animations/CustomAnimationList.cxx
b/sd/source/ui/animations/CustomAnimationList.cxx
index 196fc53..cfb8463 100644
--- a/sd/source/ui/animations/CustomAnimationList.cxx
+++ b/sd/source/ui/animations/CustomAnimationList.cxx
@@ -742,7 +742,7 @@ void CustomAnimationList::update()
 // An entry has moved down out of view; scroll down one.
 ScrollToAbsPos( nFirstVis + 1 );
 }
-else
+else if ( nFirstVis != -1 )
 {
 // The selection is still in view, or it hasn't moved.
 ScrollToAbsPos( nFirstVis );



___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] [PATCH] [PUSHED:master] Fix for Bug 37484 - On any animation change, current position in list is lost

2011-06-22 Thread Tor Lillqvist
Thanks. If these work fine on Linux (i.e. X11), they can't be awfully broken on 
other windowing systems either, I hope. Pushed to master.

--tml


___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice