Bug Fix for columns.adjust()

awelch

There is a bug in the columns.adjust() function that can cause columns to extend past the scroll bar (if using scrollY) or past the right edge of the table in some cases. This behavior will happen when using the style table-layout: fixed, allowing table cells to shrink smaller than the text they contain, and setting a column width to 0%. I have created a JS Bin showing the behavior as well as the reason it happens: live.datatables.net/rumomuze/1/edit. The reason being that setting the width of a th to 0% has a different behavior than setting the width to 0px. When the cell is set to 0px it will still be rendered, consuming an amount of space determined by it's padding, margin and border. However, when the cell is set to 0% padding, margin and border are ignored and the column is not rendered at all.

The culprit of this bug is the function _fnCalculateColumnWidths. In this function the datatable DOM instance is cloned and the columns of the cloned table are set to the column property sWidthOrig. If one of the columns sWidthOrig is 0% the cloned table's column is set to 0%, then the original table columns are calculated in px from that. This means that the px width for the other columns (those that aren't 0% width) are calculated with the 0% column not being rendered. When the calculated widths are applied to the original table the 0% column is set to 0px and suddenly has some width that didn't exist when the other columns were calculated, thus the original table is now larger than expected and extends beyond it's right boundary. The fix I am using for this involves calculating a sum of the padding, margin and border for any columns that are set to 0% and removing this space from the size of the cloned table. See the excerpt below:

/**
* Calculate the width of columns for the table
*  @param {object} oSettings dataTables settings object
*  @memberof DataTable#oApi
*/
function _fnCalculateColumnWidths ( oSettings )
{

...

// Apply custom sizing to the cloned header
headerCells = _fnGetUniqueThs(oSettings, tmpTable.find('thead')[0]);

var hiddenWidth= 0;
for (i = 0; i < visibleColumns.length; i++) {
    column = columns[visibleColumns[i]];
    if (/^0*\.?0*%$/.test(column.sWidthOrig)) {
        var header = $(column.nTh);
        hiddenWidth += header.width() === 0 ? header.outerWidth() :
            (header.css("padding-left") + header.css("padding-right") + header.css("margin-left") +
             header.css("margin-right") + header.css("border-left-width") + header.css("border-right-width"));
    }
    headerCells[i].style.width = column.sWidthOrig !== null && column.sWidthOrig !== '' ?
        _fnStringToCss(column.sWidthOrig) :
        '';

...

// When scrolling (X or Y) we want to set the width of the table as 
// appropriate. However, when not scrolling leave the table width as it
// is. This results in slightly different, but I think correct behaviour
if (scrollX && scrollXInner) {
    tmpTable.width(scrollXInner);
}
else if (scrollX) {
    tmpTable.css('width', 'auto');
    tmpTable.removeAttr('width');

    // If there is no width attribute or style, then allow the table to
    // collapse
    if (tmpTable.width() < tableContainer.clientWidth && tableWidthAttr) {
        tmpTable.width(tableContainer.clientWidth);
    }
}
else if (scrollY) {
    tmpTable.width(tableContainer.clientWidth);
}
else if (tableWidthAttr) {
    tmpTable.width(tableWidthAttr);
}

//Remove width that was displaced by columns with 0% width
tmpTable.width(tmpTable.width() - hiddenWidth);

...

}

allan

That's interesting - thanks for posting this. Using $().innerWidth() would save a little code compared to getting the CSS values (but I think not offer any performance improvement).

table-layout: fixed hasn't been extensively tested with DataTables yet, so I'm not too surprised by this. I guess box-model: border-box would also need to be considered since the width would be different in that case (although I've found column alignment issues with that before).

Let me dig into this a bit more before merging anything in .

Allan

awelch

Ah yes, outer minus inner, good call. The solution I'm working with necessitates the use of a fixed table-layout, I will try to provide all the shims I come up with to get it working right. I assume you mean box-sizing, that's not a property I've thought of changing. Any idea what value could be gained by changing from content to border box model? I believe per the CSS2 spec, before the box-sizing property, all tables were to use the border-box model (Separated borders model). I'm curious if this causes issues when using DataTables on an older browser running XHTML.

awelch

Sorry, I spoke too soon. How would you use innerWidth? Wouldn't that just get the padding (and leave out the border width)? I believe I should have used outerWidth() - width() instead of getting all the css individually.

awelch

There is a problem with my code. The line to remove the hidden width should be conditional on hiddenWidth being greater than zero. Otherwise column sizing will be messed up when using scrollX.
The correct code is:

//Remove width that was displaced by columns with 0% width
if (hiddenWidth > 0) {
tmpTable.width(tmpTable.width() - hiddenWidth);
}

allan

Yes I'm with you now. I agree about the change and not so much with my innerWidth comment!

Allan