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 212119 [details] Patch
Comment on attachment 212119 [details] Patch Attachment 212119 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/1925265 New failing tests: tables/mozilla/bugs/bug113235-1.html fast/text/international/bidi-layout-across-linebreak.html fast/table/border-collapsing/003.html fast/repaint/table-cell-collapsed-border.html fast/table/border-collapsing/003-vertical.html tables/mozilla_expected_failures/marvin/x_colgroup_width_px.xml tables/mozilla/bugs/bug14159-1.html
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
Comment on attachment 212119 [details] Patch Attachment 212119 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/2015072 New failing tests: tables/mozilla/bugs/bug113235-1.html fast/text/international/bidi-layout-across-linebreak.html fast/table/border-collapsing/003.html fast/repaint/table-cell-collapsed-border.html fast/table/border-collapsing/003-vertical.html tables/mozilla_expected_failures/marvin/x_colgroup_width_px.xml tables/mozilla/bugs/bug14159-1.html
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 212330 [details] Patch
Comment on attachment 212330 [details] Patch Attachment 212330 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/2083098 New failing tests: css3/selectors3/xhtml/css3-modsel-74b.xml css3/selectors3/xhtml/css3-modsel-28b.xml fast/css/percentage-non-integer.html css3/selectors3/xml/css3-modsel-28b.xml fast/dynamic/create-renderer-for-whitespace-only-text.html fast/block/positioning/053.html accessibility/table-cells.html css3/selectors3/xml/css3-modsel-28.xml fast/css/css3-nth-child.html css3/selectors3/html/css3-modsel-28b.html css3/selectors3/xml/css3-modsel-74.xml css3/selectors3/html/css3-modsel-18b.html css3/selectors3/xml/css3-modsel-18b.xml fast/css/acid2-pixel.html css3/selectors3/xhtml/css3-modsel-18.xml css3/selectors3/html/css3-modsel-28.html css3/selectors3/xhtml/css3-modsel-18b.xml css3/selectors3/html/css3-modsel-18.html fast/css/acid2.html css3/selectors3/html/css3-modsel-74.html css3/selectors3/xml/css3-modsel-74b.xml fast/dynamic/insert-before-table-part-in-continuation.html fast/dom/HTMLTableElement/colSpan.html css3/selectors3/xhtml/css3-modsel-74.xml fast/forms/basic-buttons.html editing/pasteboard/paste-table-001.html css3/selectors3/html/css3-modsel-74b.html css3/selectors3/xhtml/css3-modsel-28.xml css3/selectors3/xml/css3-modsel-18.xml css2.1/20110323/table-caption-002.htm
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
Comment on attachment 212330 [details] Patch Attachment 212330 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/2095062 New failing tests: css3/selectors3/xhtml/css3-modsel-74b.xml css3/selectors3/xhtml/css3-modsel-28b.xml fast/css/percentage-non-integer.html css3/selectors3/xml/css3-modsel-28b.xml fast/dynamic/create-renderer-for-whitespace-only-text.html fast/block/positioning/053.html accessibility/table-cells.html css3/selectors3/xml/css3-modsel-28.xml fast/css/css3-nth-child.html css3/selectors3/html/css3-modsel-28b.html css3/selectors3/xml/css3-modsel-74.xml css3/selectors3/html/css3-modsel-18b.html css3/selectors3/xml/css3-modsel-18b.xml fast/css/acid2-pixel.html css3/selectors3/xhtml/css3-modsel-18.xml css3/selectors3/html/css3-modsel-28.html css3/selectors3/xhtml/css3-modsel-18b.xml css3/selectors3/html/css3-modsel-18.html fast/css/acid2.html css3/selectors3/html/css3-modsel-74.html css3/selectors3/xml/css3-modsel-74b.xml fast/dynamic/insert-before-table-part-in-continuation.html fast/dom/HTMLTableElement/colSpan.html css3/selectors3/xhtml/css3-modsel-74.xml fast/forms/basic-buttons.html editing/pasteboard/paste-table-001.html css3/selectors3/html/css3-modsel-74b.html css3/selectors3/xhtml/css3-modsel-28.xml css3/selectors3/xml/css3-modsel-18.xml css2.1/20110323/table-caption-002.htm
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 212997 [details] Patch
Comment on attachment 212997 [details] Patch Attachment 212997 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/2695168 New failing tests: tables/mozilla/marvin/body_col.html fast/table/021.html fast/text/international/bidi-layout-across-linebreak.html tables/mozilla/core/one_row.html tables/mozilla/other/body_col.html fast/table/border-collapsing/cached-change-col-border-width.html tables/mozilla_expected_failures/bugs/bug14007-2.html fast/table/border-collapsing/cached-cell-remove.html tables/mozilla_expected_failures/marvin/x_colgroup_width_px.xml tables/mozilla_expected_failures/core/col_span2.html fast/table/border-collapsing/cached-change-colgroup-border-width.html fast/table/border-collapsing/cached-change-cell-border-width.html fast/table/border-collapsing/cached-cell-append.html fast/table/border-collapsing/cached-change-table-border-width.html
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
Comment on attachment 212997 [details] Patch Attachment 212997 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/2816101 New failing tests: tables/mozilla/marvin/body_col.html fast/table/021.html fast/text/international/bidi-layout-across-linebreak.html tables/mozilla/core/one_row.html tables/mozilla/other/body_col.html fast/table/border-collapsing/cached-change-col-border-width.html tables/mozilla_expected_failures/bugs/bug14007-2.html fast/table/border-collapsing/cached-cell-remove.html tables/mozilla_expected_failures/marvin/x_colgroup_width_px.xml tables/mozilla_expected_failures/core/col_span2.html fast/table/border-collapsing/cached-change-colgroup-border-width.html fast/table/border-collapsing/cached-change-cell-border-width.html fast/table/border-collapsing/cached-cell-append.html fast/table/border-collapsing/cached-change-table-border-width.html
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
Comment on attachment 212997 [details] Patch Attachment 212997 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/2720226 New failing tests: tables/mozilla/marvin/body_col.html fast/table/021.html fast/text/international/bidi-layout-across-linebreak.html tables/mozilla/core/one_row.html tables/mozilla/other/body_col.html fast/table/border-collapsing/cached-change-col-border-width.html tables/mozilla_expected_failures/bugs/bug14007-2.html fast/table/border-collapsing/cached-cell-remove.html tables/mozilla_expected_failures/marvin/x_colgroup_width_px.xml tables/mozilla_expected_failures/core/col_span2.html fast/table/border-collapsing/cached-change-colgroup-border-width.html fast/table/border-collapsing/cached-change-cell-border-width.html fast/table/border-collapsing/cached-cell-append.html fast/table/border-collapsing/cached-change-table-border-width.html
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
Created attachment 213155 [details] Patch
(In reply to comment #20) > Created an attachment (id=213155) [details] > Patch Can someone please review this?
(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 #22) > (In reply to comment #21) > > (In reply to comment #20) > > > Created an attachment (id=213155) [details] [details] [details] > > > Patch > > > > Can someone please review this? > > Hi. Can someone please review this. Thanks Please review.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 221123 [details] Patch
Comment on attachment 221123 [details] Patch Attachment 221123 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6538699684708352 New failing tests: fast/dom/HTMLTableColElement/resize-table-width-using-col-width.html
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
Comment on attachment 221123 [details] Patch Attachment 221123 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5507948201639936 New failing tests: fast/dom/HTMLTableColElement/resize-table-width-using-col-width.html
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
Comment on attachment 221123 [details] Patch Attachment 221123 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5231997626613760 New failing tests: fast/dom/HTMLTableColElement/resize-table-width-using-col-width.html
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 221143 [details] Patch
Comment on attachment 221143 [details] Patch Attachment 221143 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5729575493435392 New failing tests: fast/dom/HTMLTableColElement/resize-table-width-using-col-width.html
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
Comment on attachment 221143 [details] Patch Attachment 221143 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5217174083862528 New failing tests: fast/dom/HTMLTableColElement/resize-table-width-using-col-width.html
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
Comment on attachment 221143 [details] Patch Attachment 221143 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/4810267405844480 New failing tests: fast/dom/HTMLTableColElement/resize-table-width-using-col-width.html
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
Created attachment 221227 [details] Patch
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 221526 [details] Patch
Created attachment 221608 [details] Patch
Comment on attachment 221608 [details] Patch Attachment 221608 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5820269700579328 New failing tests: fast/dom/HTMLTableColElement/resize-table-width-using-col-width.html
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
Comment on attachment 221608 [details] Patch Attachment 221608 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5459725986562048 New failing tests: fast/dom/HTMLTableColElement/resize-table-width-using-col-width.html
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
Created attachment 221613 [details] Patch
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.
Created attachment 221633 [details] Patch
Comment on attachment 221633 [details] Patch Clearing flags on attachment: 221633 Committed r162334: <http://trac.webkit.org/changeset/162334>
All reviewed patches have been landed. Closing bug.
(In reply to comment #52) > (From update of attachment 221633 [details]) > Clearing flags on attachment: 221633 > > Committed r162334: <http://trac.webkit.org/changeset/162334> This caused a regression fixed in r165837: <http://trac.webkit.org/changeset/165837>