[PATCH] proposed fix for fdo#44283

classic Classic list List threaded Threaded
4 messages Options
julien2412 julien2412
Reply | Threaded
Open this post in threaded view
|

[PATCH] proposed fix for fdo#44283

Hello,

The bug is : "tabulator does not change row in tables"
First, let's have in mind the code of getColumnId (svtools/source/brwbox/brwbox2.cxx) :
    481 sal_uInt16 BrowseBox::GetColumnId( sal_uInt16 nPos ) const
    482 {
    483     DBG_CHKTHIS(BrowseBox,BrowseBoxCheckInvariants);
    484
    485     if ( nPos >= pCols->size() )
    486         return BROWSER_INVALIDID;
    487     return (*pCols)[ nPos ]->GetId();
    488 }
So if want to go further than max row, it returns BROWSER_INVALIDID (equals to SAL_MAX_UINT16)

So I think the problem is here  (still in brwbox2.cxx) :
    1999         case BROWSER_CURSORRIGHT:
   2000             if ( bColumnCursor )
   2001             {
   2002                 sal_uInt16 nNewPos = GetColumnPos( GetCurColumnId() ) + 1;
   2003                 sal_uInt16 nNewId = GetColumnId( nNewPos );
   2004                 if (nNewId != 0)    // Am Zeilenende ?  <-- we should compare nNewId with BROWSER_INVALIDID not with 0
   2005                     bDone = GoToColumnId( nNewId );
   2006                 else
   2007                 {
   2008                     sal_uInt16 nColId = ( GetColumnId(0) == 0 ) ? GetColumnId(1) : GetColumnId(0);
   2009                     if ( GetRowCount() )
   2010                         bDone = ( nCurRow < GetRowCount() - 1 ) && GoToRowColumnId( nCurRow + 1, nColId );
   2011                     else if ( ColCount() )
   2012                         GoToColumnId( nColId );
   2013                 }
   2014             }
   2015             else
   2016                 bDone = ScrollColumns( 1 ) != 0;
   2017             break;

But here is the code of getColumnId (still in brwbox2.cxx) :
    481 sal_uInt16 BrowseBox::GetColumnId( sal_uInt16 nPos ) const
    482 {
    483     DBG_CHKTHIS(BrowseBox,BrowseBoxCheckInvariants);
    484
    485     if ( nPos >= pCols->size() )
    486         return BROWSER_INVALIDID;
    487     return (*pCols)[ nPos ]->GetId();
    488 }
So if want to go further than max row, it returns BROWSER_INVALIDID (equals to SAL_MAX_UINT16)

I replaced 0 by BROWSER_INVALIDID, it compiles and seems to work.

I propose this patch : patch.txt

Is it ok for you ? If yes I can commit and push on master (3.5 too ?)

Julien
Caolán McNamara Caolán McNamara
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] proposed fix for fdo#44283

On Tue, 2012-01-10 at 13:38 -0800, julien2412 wrote:
> I replaced 0 by BROWSER_INVALIDID, it compiles and seems to work.
>
> I propose this patch :
> http://nabble.documentfoundation.org/file/n3648948/patch.txt patch.txt

Reading your patch and also
http://cgit.freedesktop.org/libreoffice/core/commit/?id=f7ed67f2cf79fd067c1634e8f7c3f8a17792f2e5

"GetColumnId: return BROWSER_INVALIDID, not 0 (== id of handle column)
for an invalid column; adapt code calling GetColumnId to this change."

It looks like you're right at this was a missed adaption of the
GetColumnId change.

> Is it ok for you ? If yes I can commit and push on master (3.5 too ?)

Looks good to me on the surface anyway. Go for it I say for master and
3.5. Lionel ? are we right here ?

C.

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Lionel Élie Mamane Lionel Élie Mamane
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] proposed fix for fdo#44283

On Wed, Jan 11, 2012 at 04:22:59PM +0000, Caolán McNamara wrote:
> On Tue, 2012-01-10 at 13:38 -0800, julien2412 wrote:

>> I replaced 0 by BROWSER_INVALIDID, it compiles and seems to work.

>> I propose this patch :
>> http://nabble.documentfoundation.org/file/n3648948/patch.txt patch.txt

> It looks like you're right at this was a missed adaption of the
> GetColumnId change.

> Looks good to me on the surface anyway. Go for it I say for master and
> 3.5. Lionel ? are we right here ?

Yes, that patch is right. I committed (and pushed) a slightly extended
version of it to libreoffice-3-5 and a more extensive cleanup to
master.

Thanks Julien!

--
Lionel
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Ivan Timofeev Ivan Timofeev
Reply | Threaded
Open this post in threaded view
|

Re: [PUSHED][PATCH] proposed fix for fdo#44283

11.01.2012 22:15, Lionel Elie Mamane пишет:
> On Wed, Jan 11, 2012 at 04:22:59PM +0000, Caolán McNamara wrote:
>> Looks good to me on the surface anyway. Go for it I say for master and
>> 3.5. Lionel ? are we right here ?
>
> Yes, that patch is right. I committed (and pushed) a slightly extended
> version of it to libreoffice-3-5 and a more extensive cleanup to
> master.

[PUSHED]

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice