[Libreoffice] [REVIEW] fdo#40701 Base crashes when "Find Record" button is clicked

The attached patches are backport to 3.4 of my master commits
b71d12fbfdd4aad0b576645a0d7bbf71d1da7a3a
0ef7ce4a234c6649dcca219b96185acb7634fe34
1b6310433280ae4e8439dcbf17dfa763bf2826cc

They are related to fdo#40701.

0001-fdo-40701-DbGridControl-RemoveColumn-even-if-no-corr.patch
fixes the root cause of the bug, which caused
void DbGridControl::EnableHandle(sal_Bool bEnable)
{
    RemoveColumn(0);
    m_bHandle = bEnable;
    InsertHandleColumn();
}
to misfunction: RemoveColumn(0) silently did not remove the column, so
the call to InsertHandleColumn() added a second Handle Column, which
confused the code in other places.

0002-FmXGridPeer-getByIndex-Error-checking-of-pGrid-GetMo.patch
fixes the reason the bug was a crasher instead of just "broken
feature, nothing happens when "Find Record" button is clicked".

0003-Don-t-touch-handle-when-setting-property-Enabled.patch
is not per se directly related to fdo#40701, but it looks like a
(subtle) bug waiting to be discovered: The code enables/disables the
handle when the property "Enabled" is set, according to the value of
the "Enabled" property, while should do so (only) when setting the
property RecordMarker, according to its value. If one looks at the
commit message of 448cd2de7919401a0a84cd6e5dd5ede276ddf28e and its
diff, it looks rather strongly like this was *not* an intended change,
as that commit looks like a janitorial commit that did not intend to
change behaviour of the code.

As such, my case for
0003-Don-t-touch-handle-when-setting-property-Enabled.patch
in libreoffice-3-4 is much weaker, but I'm throwing it in and let's
see what you guys think of it.

-- 
Lionel

From f6a89eaf00a6230ddbfbb8bc5a09cc0f32b519af Mon Sep 17 00:00:00 2001
From: Lionel Elie Mamane <lionel@mamane.lu>
Date: Mon, 12 Sep 2011 00:00:33 +0200
Subject: [PATCH 1/3] fdo#40701: DbGridControl::RemoveColumn even if no corresponding Model column

That case crops up when nId==0 i.e. the Handle column
---
 svx/source/fmcomp/gridctrl.cxx |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/svx/source/fmcomp/gridctrl.cxx b/svx/source/fmcomp/gridctrl.cxx
index bb666bc..aec3738 100644
--- a/svx/source/fmcomp/gridctrl.cxx
+++ b/svx/source/fmcomp/gridctrl.cxx
@@ -1723,11 +1723,12 @@ sal_uInt16 DbGridControl::AppendColumn(const XubString& rName, sal_uInt16 
nWidth
 void DbGridControl::RemoveColumn(sal_uInt16 nId)
 {
     sal_uInt16 nIndex = GetModelColumnPos(nId);
-    if (nIndex == GRID_COLUMN_NOT_FOUND)
-        return;
 
     DbGridControl_Base::RemoveColumn(nId);
 
+    if (nIndex == GRID_COLUMN_NOT_FOUND)
+        return;
+
     delete m_aColumns[ nIndex ];
     DbGridColumns::iterator it = m_aColumns.begin();
     ::std::advance( it, nIndex );
-- 
1.7.2.5

From 9e34b69c3db910c51a910c8be3df9c740541fb12 Mon Sep 17 00:00:00 2001
From: Lionel Elie Mamane <lionel@mamane.lu>
Date: Sun, 11 Sep 2011 23:40:09 +0200
Subject: [PATCH 2/3] FmXGridPeer::getByIndex: Error checking of pGrid->GetModelColumnPos(nId) call

Fixes crash of fdo#40701, but not broken feature
---
 svx/source/fmcomp/fmgridif.cxx |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/svx/source/fmcomp/fmgridif.cxx b/svx/source/fmcomp/fmgridif.cxx
index 3080d77..a4146a9 100644
--- a/svx/source/fmcomp/fmgridif.cxx
+++ b/svx/source/fmcomp/fmgridif.cxx
@@ -2442,6 +2442,9 @@ Any FmXGridPeer::getByIndex(sal_Int32 _nIndex) throw( 
IndexOutOfBoundsException,
     // get the list position
     sal_uInt16 nPos = pGrid->GetModelColumnPos(nId);
 
+    if ( nPos == GRID_COLUMN_NOT_FOUND )
+        return aElement;
+
     DbGridColumn* pCol = pGrid->GetColumns().at( nPos );
     Reference< ::com::sun::star::awt::XControl >  xControl(pCol->GetCell());
     aElement <<= xControl;
-- 
1.7.2.5

From 17f18e8d57639f000565cdb57468b983f2412474 Mon Sep 17 00:00:00 2001
From: Lionel Elie Mamane <lionel@mamane.lu>
Date: Sun, 11 Sep 2011 23:44:41 +0200
Subject: [PATCH 3/3] Don't touch handle when setting property Enabled

Handle enabling/disabling is controlled by property RecordMarker,
not by property Enabled.

Source of the error is most probably a copy/paste error in
commit 448cd2de7919401a0a84cd6e5dd5ede276ddf28e
Author: Frank Schoenheit [fs] <frank.schoenheit@oracle.com>
Date:   Tue Nov 23 11:38:49 2010 +0100
---
 svx/source/fmcomp/fmgridif.cxx |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/svx/source/fmcomp/fmgridif.cxx b/svx/source/fmcomp/fmgridif.cxx
index a4146a9..222b2b5 100644
--- a/svx/source/fmcomp/fmgridif.cxx
+++ b/svx/source/fmcomp/fmgridif.cxx
@@ -2090,7 +2090,6 @@ void FmXGridPeer::setProperty( const ::rtl::OUString& PropertyName, const 
Any& V
     {
         sal_Bool bValue( sal_True );
         OSL_VERIFY( Value >>= bValue );
-        pGrid->EnableHandle( bValue );
 
         // Im DesignModus nur das Datenfenster disablen
         // Sonst kann das Control nicht mehr konfiguriert werden
-- 
1.7.2.5