DbGridControl's confusion of BrowseBox's column number vs. column ID

[Stephan Bergmann]

svtools' BrowseBox class (include/svtools/brwbox.hxx; effectively a 
collection of BrowseColumn via the BrowseBox::pCols member) apparently 
distinguishes between a column's index/position/number (i.e., the 
BrowseColumn's position within the BrowseBox::pCols vector) and its ID 
(i.e., the BrowseColumn::_nId member, svtools/source/brwbox/datwin.hxx).

Columns are inserted into a BrowseBox with (both in 
svtools/source/brwbox/brwbox1.cxx:) BrowseBox::InsertHandleColumn 
(inserting a "handle" column at position 0 with ID 0) and 
BrowseBox::InsertDataColumn (inserting a "data" column with a given 
nItemId somewhere in the collection).

Historically both column numbers and column IDs have been represented 
with unsigned 16-bit integers (both sal_uInt16, by now), which doesn't 
necessarily help in avoiding confusion between the two concepts.  (And 
indeed e.g. BROWSER_INVALIDID = SAL_MAX_UNIT16 appears to be used to 
represent both "no column ID", BrowseBox::GetColumnId, 
svtools/source/brwbox/brwbox2.cxx, and "no column number", 
BrowseBox::GetColumnAtXPosPixel, svtools/source/brwbox/brwbox1.cxx).

Now, BrowseBox::GetColumnAtXPosPixel (svtools/source/brwbox/brwbox1.cxx) 
apparently always returned a column number (from 
<https://cgit.freedesktop.org/libreoffice/core/commit/?id=8ab086b6cc054501bfbf7ef6fa509c393691e860> 
"initial import"),

> USHORT BrowseBox::GetColumnAtXPosPixel( long nX, BOOL ) const
> {
>     DBG_CHKTHIS(BrowseBox,BrowseBoxCheckInvariants);
>
>     // accumulate the withds of the visible columns
>     long nColX = 0;
>     USHORT nCol;
>     for ( nCol = 0; nCol < USHORT(pCols->Count()); ++nCol )
>     {
>         BrowserColumn *pCol = pCols->GetObject(nCol);
>         if ( pCol->IsFrozen() || nCol >= nFirstCol )
>             nColX += pCol->Width();
>
>         if ( nColX > nX )
>             return nCol;
>     }
>
>     return BROWSER_INVALIDID;
> }

while DbGridControl::GetModelColumnPos (svx/source/fmcomp/gridctrl.cxx) 
apparently always expected a column ID as input (from 
<http://cgit.freedesktop.org/libreoffice/core/commit/?id=fd069bee7e57ad529c3c0974559fd2d84ec3151a> 
"initial import"),

> sal_uInt16 DbGridControl::GetModelColumnPos( sal_uInt16 nId ) const
> {
>     for (sal_uInt16 i=0; i<m_aColumns.Count(); ++i)
>         if (m_aColumns.GetObject(i)->GetId() == nId)
>             return i;
>
>     return -1;
> }

But there is two occurrences of code in svx/source/fmcomp/gridctrl.cxx 
that feeds the output of GetColumnAtXPosPixel as input into 
GetModelColumnPos, i.e., misinterprets a column number as a column ID. 
Meanwhile that code lives in DbGridControl::StartDrag,

[...]
>     sal_uInt16 nColId = GetColumnAtXPosPixel(rPosPixel.X());
>     long   nRow = GetRowAtYPosPixel(rPosPixel.Y());
>     if (nColId != HandleColumnId && nRow >= 0)
>     {
>         if (GetDataWindow().IsMouseCaptured())
>             GetDataWindow().ReleaseMouse();
>
>         size_t Location = GetModelColumnPos( nColId );
[...]

and DbGridControl::Command,

[...]
>             sal_uInt16 nColId = GetColumnAtXPosPixel(rEvt.GetMousePosPixel().X());
>             long   nRow = GetRowAtYPosPixel(rEvt.GetMousePosPixel().Y());
>
>             if (nColId == HandleColumnId)
>             {
>                 executeRowContextMenu( nRow, rEvt.GetMousePosPixel() );
>             }
>             else if (canCopyCellText(nRow, nColId))
>             {
>                 ScopedVclPtrInstance<PopupMenu> aContextMenu(SVX_RES(RID_SVXMNU_CELL));
>                 aContextMenu->RemoveDisabledEntries(true, true);
>                 switch (aContextMenu->Execute(this, rEvt.GetMousePosPixel()))
>                 {
>                     case SID_COPY:
>                         copyCellText(nRow, nColId);
[...]

where DbGridControl::copyCellText (svx/source/fmcomp/gridctrl.cxx) calls 
GetModelColumnPos,

> void DbGridControl::copyCellText(sal_Int32 _nRow, sal_uInt16 _nColId)
> {
>     DBG_ASSERT(canCopyCellText(_nRow, _nColId), "DbGridControl::copyCellText: invalid call!");
>     DbGridColumn* pColumn = m_aColumns[ GetModelColumnPos(_nColId) ];
>     SeekRow(_nRow);
>     OStringTransfer::CopyString( GetCurrentRowCellText( pColumn,m_xPaintRow ), this );
> }

Does anybody (Linoel?) known something about that DbGridControl code? 
Would this be a bug (if so, any idea how to reproduce it at the GUI?) or 
is it maybe an invariant of such a DbGridControl that it's columns' ID 
always match the columns' positions?