Created attachment 161604 [details] Test case The current table logic wrongly handles empty tables and still allocates some width for empty tables (ie tables without any cells). This can be seen in our existing tests: * http://trac.webkit.org/browser/trunk/LayoutTests/platform/chromium-win/fast/table/inline-form-assert-expected.txt (final table) * http://trac.webkit.org/browser/trunk/LayoutTests/platform/chromium-win/tables/mozilla/bugs/bug113235-2-expected.txt * http://trac.webkit.org/browser/trunk/LayoutTests/platform/chromium-win/tables/mozilla/bugs/bug56405-expected.txt (first table) There are several bugs leading to this bug: border-spacing is added even if we have no cells (see RenderTable::bordersPaddingAndSpacingInRowDirection) but also because the column counting logic doesn't work well (sections have one column by default regardless of the content).
Changing the title to 'without any descendant' as 'border-spacing' can make a table have a logical width if we have a row / row-group. Whether to apply 'border-spacing' to empty rows is something browsers don't agree on so I don't want to touch it.
Created attachment 162150 [details] Proposed change: fixed the bug + rebaseline.
Comment on attachment 162150 [details] Proposed change: fixed the bug + rebaseline. Attachment 162150 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13754820 New failing tests: fast/invalid/018.html fast/invalid/table-residual-style-crash.html fast/forms/file/file-input-disabled.html fast/invalid/017.html fast/invalid/020.xml fast/invalid/residual-style.html fast/invalid/table-inside-stray-table-content.html
Created attachment 162269 [details] Proposed change #2, with moar rebaselining.
Comment on attachment 162269 [details] Proposed change #2, with moar rebaselining. View in context: https://bugs.webkit.org/attachment.cgi?id=162269&action=review > Source/WebCore/ChangeLog:9 > + width even if we didn't have a column. Please add information on how we match Firefox and Opera, and bug in IE. > Source/WebCore/rendering/RenderTable.h:185 > + return (effectiveColumnCount + 1) * hBorderSpacing(); Please do the static_cast<LayoutUnit>(numEffCols() + 1). We don't want to overflow here since it wont use the code path of saturated arithmetic in fractional layout unit.
Created attachment 162864 [details] Patch for landing
Comment on attachment 162864 [details] Patch for landing Rejecting attachment 162864 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: eCellElement.o] Error 1 make: *** Waiting for unfinished jobs.... cc1plus: warnings being treated as errors In file included from Source/WebCore/html/HTMLTableElement.cpp:41: Source/WebCore/rendering/RenderTable.h: In member function 'WebCore::LayoutUnit WebCore::RenderTable::borderSpacingInRowDirection() const': Source/WebCore/rendering/RenderTable.h:184: error: unused variable 'effectiveColumnCount' make: *** [out/Release/obj.target/webcore_html/Source/WebCore/html/HTMLTableElement.o] Error 1 Full output: http://queues.webkit.org/results/13785604
Created attachment 162871 [details] Patch for landing
Comment on attachment 162871 [details] Patch for landing Removing the patch from the cq as I forgot at least another rebaselining in fast/forms/file/file-input-disabled.html. Letting the EWS run for real.
Created attachment 162929 [details] Patch for the EWS. Only change is LayoutTests/fast/forms/file/file-input-disabled.html (empty table progression).
Created attachment 164583 [details] Patch for landing
Comment on attachment 164583 [details] Patch for landing Clearing flags on attachment: 164583 Committed r128906: <http://trac.webkit.org/changeset/128906>
All reviewed patches have been landed. Closing bug.