Incorrect behaviour of TableHeaderComponent::setColumnWidth

#1

Hi all

It looks like there are two bugs in the existing code which result in incorrect behavior of TableHeaderComponent. It’s about the following piece of code from setColumnWidth function:

const int index = getIndexOfColumnId (columnId, true) + 1;
if (index < getNumColumns (true))
{
	     const int x = getColumnPosition (index).getX();
           resizeColumnsToFit (index, getWidth() - x);
}

When you try to resize the last column to get it smaller, there is no visible column next to the column being resized. So, getIndexOfColumnId() will return -1 and passing -1 to resizeColumnsToFit will result in not resizing of the current column to fit the total width. I think the existing code:

const int index = getIndexOfColumnId (columnId, true) + 1;

should be

// Resize the current column too
const int index = getIndexOfColumnId (columnId, true);

Further, resizeColumnsToFit function operates on absolute indexes of columns and you have to pass an absolute index to the function instead of visible-indexes of visible columns. So changing the call of resizeColumnsToFit to something like

int id = getColumnIdOfIndex(index, true);
resizeColumnsToFit (getIndexOfColumnId(id, false), getWidth() - x);

should fix this problem.

#2

Thanks for that. I don’t think you’re quite right though. If you’re in stretch-to-fit mode, then you shouldn’t be resizing the last column at all, as its right-hand edge is supposed to always be in the same position. The correct way to make it smaller is to make one or more of the previous columns wider, depending on the effect you want.

But you’re right that there are a couple of bugettes in there. I think fixes should be:

in setColumnWidth():

[code] if (stretchToFit)
{
const int index = getIndexOfColumnId (columnId, true) + 1;

        if (index >= 0 && index < getNumColumns (true))
        {
            const int x = getColumnPosition (index).getX();
            resizeColumnsToFit (visibleIndexToTotalIndex (index), getWidth() - x);
        }
    }[/code]

and in mouseDrag():

[code] if (columnIdBeingResized != 0)
{
const ColumnInfo* const ci = getInfoForId (columnIdBeingResized);

    if (ci != 0)
    {
        int w = jlimit (ci->minimumWidth, ci->maximumWidth,
                        initialColumnWidth + e.getDistanceFromDragStartX());

        if (stretchToFit)
        {
            // prevent us dragging a column too far right if we're in stretch-to-fit mode
            int minWidthOnRight = 0;
            for (int i = getIndexOfColumnId (columnIdBeingResized, false) + 1; i < columns.size(); ++i)
                if (columns.getUnchecked (i)->isVisible())
                    minWidthOnRight += columns.getUnchecked (i)->minimumWidth;

            const Rectangle currentPos (getColumnPosition (getIndexOfColumnId (columnIdBeingResized, true)));
            w = jmax (ci->minimumWidth, jmin (w, getWidth() - minWidthOnRight - currentPos.getX()));
        }

        setColumnWidth (columnIdBeingResized, w);
    }
}[/code]
#3

If there is no column with the given id, getIndexOfColumnId will return -1, hence index will be zero in the previous code.
This will lead to bad column being resized (in fact the first one while none should have been).

#4

Well, no - at that point in the code, there’s always guaranteed to be a column with that ID, so it can never return -1.

#5