In the example on the attached URL, a simple table with one column and one row is used. The width of this table is controlled by the width of the single col element. If the col width is dynamically updated to a wider width than the original width, the table resizes just fine, but if the col width is updated to a narrower width than the original width, the table will not get narrower than the original width.
The expected behavior is that the table width would shrink as well as grow as the col width changes.
The same behavior was observed with both an auto and fixed layout table.
This behavior was observed in Chrome 23.0.1271.97 and Safari 5.1.7 on Windows 7 as well as Chrome 23.0.1271.97 and Safari 6.0.2 on OS X 10.8.
The expected behavior was observed in Firefox 17.0.1, IE 9.0.8112.16421 and Opera 12.11.1661 on Windows 7 as well as Firefox 17.0.1 on OS X 10.8.
Created attachment 212124[details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Created attachment 212125[details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Created attachment 212333[details]
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-10 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Created attachment 212334[details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Hi Julien. I am working on this issue but had one doubt.
As per the content initial width is set to 200px. When the width changes the table width is not set to the new values because
void AutoTableLayout::recalcColumn(unsigned effCol)
if (cell->maxPreferredLogicalWidth() > columnLayout.maxLogicalWidth) {
columnLayout.maxLogicalWidth = cell->maxPreferredLogicalWidth();
maxContributor = cell;
}
cell->maxPreferredLogicalWidth is still 200 and columnLayout.maxLogicalWidth is the new for example 180 (reduce by 20px) and since the condition doesnot satisfy the new width 180px is not set to columnLayout.maxLogicalWidth.
So is it better to do some workaround for this in void AutoTableLayout::recalcColumn(unsigned effCol) or calculates the preflogicalwidths again?
I tried the first way but some test cases are failing
Please suggest.
(In reply to comment #11)
> Hi Julien. I am working on this issue but had one doubt.
> As per the content initial width is set to 200px. When the width changes the table width is not set to the new values because
>
> void AutoTableLayout::recalcColumn(unsigned effCol)
>
> if (cell->maxPreferredLogicalWidth() > columnLayout.maxLogicalWidth) {
> columnLayout.maxLogicalWidth = cell->maxPreferredLogicalWidth();
> maxContributor = cell;
> }
> cell->maxPreferredLogicalWidth is still 200 and columnLayout.maxLogicalWidth is the new for example 180 (reduce by 20px) and since the condition doesnot satisfy the new width 180px is not set to columnLayout.maxLogicalWidth.
>
> So is it better to do some workaround for this in void AutoTableLayout::recalcColumn(unsigned effCol) or calculates the preflogicalwidths again?
>
> I tried the first way but some test cases are failing
>
> Please suggest.
Someone Please suggest.
Created attachment 213011[details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Created attachment 213013[details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Created attachment 213017[details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04 Port: mac-mountainlion Platform: Mac OS X 10.8.5
(In reply to comment #21)
> (In reply to comment #20)
> > Created an attachment (id=213155) [details] [details]
> > Patch
>
> Can someone please review this?
Hi. Can someone please review this. Thanks
(In reply to comment #23)
> (In reply to comment #22)
> > (In reply to comment #21)
> > > (In reply to comment #20)
> > > > Created an attachment (id=213155) [details] [details] [details] [details]
> > > > Patch
> > >
> > > Can someone please review this?
> >
> > Hi. Can someone please review this. Thanks
>
> Please review.Thanks
Hi Simon. Can you please review this patch?
Comment on attachment 213155[details]
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=213155&action=review> Source/WebCore/rendering/AutoTableLayout.cpp:85
> columnLayout.minLogicalWidth = max<int>(cell->minPreferredLogicalWidth(), columnLayout.minLogicalWidth);
> +
> + Length cellLogicalWidth = cell->styleOrColLogicalWidth();
> + if (cellLogicalWidth.isFixed() && cell->style()->logicalWidth().isAuto()) {
> + int logicalWidth = cell->adjustBorderBoxLogicalWidthForBoxSizing(cellLogicalWidth.value());
> + if (cellLogicalWidth.value() > cell->minPreferredLogicalWidth() && cell->maxPreferredLogicalWidth() != logicalWidth)
> + cell->setPreferredLogicalWidthsDirty(true);
> + }
Actually this looks wrong. The code saves minPreferredLogicalWidth to columnLayout.minLogicalWidth and then marks logical width dirty.
Also I suspect the invalidation should be done when the col width changes rather than during recalcColumn (which happens during layout).
Created attachment 221135[details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Created attachment 221136[details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Created attachment 221138[details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Created attachment 221148[details]
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-16 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Created attachment 221154[details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Created attachment 221157[details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 221227[details]
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=221227&action=review> Source/WebCore/rendering/RenderTableCol.cpp:58
> + unsigned nEffCols = table->numEffCols();
> + for (unsigned j = 0; j < nEffCols; j++) {
Ideally this would be optimized by figuring out which columns this RenderTableCol actually affects. This may not be critical to do now as animating <col> width is likely rare.
> Source/WebCore/rendering/RenderTableCol.cpp:62
> + for (RenderObject* child = table->firstChild(); child; child = child->nextSibling()) {
> + if (child->isTableSection()) {
> + RenderTableSection* section = toRenderTableSection(child);
> + unsigned numRows = section->numRows();
You should use C++11 and child iterators here:
for (auto& section : childrenOfType<RenderTableSection>(*table)) {
unsigned rowCount = section.numRows()
....
> Source/WebCore/rendering/RenderTableCol.cpp:66
> + RenderTableSection::CellStruct current = section->cellAt(i, j);
> + RenderTableCell* cell = current.primaryCell();
> + cell->setPreferredLogicalWidthsDirty(true);
You can just use section.primaryCellAt(i, j)
Are you sure the cell can't be null? Looking around it is always null tested in similar cases.
Created attachment 221609[details]
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-16 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Created attachment 221612[details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 221613[details]
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=221613&action=review> Source/WebCore/rendering/RenderTableCol.cpp:63
> + unsigned nEffCols = table->numEffCols();
> + for (unsigned j = 0; j < nEffCols; j++) {
> + for (auto& section : childrenOfType<RenderTableSection>(*table)) {
> + unsigned rowCount = section.numRows();
> + for (unsigned i = 0; i < rowCount; i++) {
> + RenderTableCell* cell = section.primaryCellAt(i, j);
It is bit strange (and inefficient) to iterate over columns and then over sections. Could you make the section loop the outermost one?
(In reply to comment #49)
> (From update of attachment 221613[details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=221613&action=review
>
> > Source/WebCore/rendering/RenderTableCol.cpp:63
> > + unsigned nEffCols = table->numEffCols();
> > + for (unsigned j = 0; j < nEffCols; j++) {
> > + for (auto& section : childrenOfType<RenderTableSection>(*table)) {
> > + unsigned rowCount = section.numRows();
> > + for (unsigned i = 0; i < rowCount; i++) {
> > + RenderTableCell* cell = section.primaryCellAt(i, j);
>
> It is bit strange (and inefficient) to iterate over columns and then over sections. Could you make the section loop the outermost one?
Just one query in void AutoTableLayout::fullRecalc() it recalcColumn which internally gets the cells from section. Something similiar I was trying to do.
2013-09-19 20:49 PDT, gur.trio
2013-09-19 21:36 PDT, Build Bot
2013-09-19 22:01 PDT, Build Bot
2013-09-23 00:39 PDT, gur.trio
2013-09-23 01:20 PDT, Build Bot
2013-09-23 01:33 PDT, Build Bot
2013-09-30 08:28 PDT, gur.trio
2013-09-30 10:18 PDT, Build Bot
2013-09-30 10:51 PDT, Build Bot
2013-09-30 11:38 PDT, Build Bot
2013-10-02 01:15 PDT, gur.trio
2014-01-14 01:42 PST, gur.trio
2014-01-14 02:37 PST, Build Bot
2014-01-14 02:57 PST, Build Bot
2014-01-14 03:56 PST, Build Bot
2014-01-14 05:05 PST, gur.trio
2014-01-14 05:40 PST, Build Bot
2014-01-14 06:26 PST, Build Bot
2014-01-14 06:36 PST, Build Bot
2014-01-14 20:37 PST, gur.trio
2014-01-17 20:26 PST, gur.trio
2014-01-19 20:48 PST, gur.trio
2014-01-19 21:45 PST, Build Bot
2014-01-19 22:00 PST, Build Bot
2014-01-19 22:09 PST, gur.trio
2014-01-20 01:18 PST, gur.trio