Created attachment 106947 [details]
tables that have either a <thead> *or* <tfoot> + a <tbody> where all rows are hidden (display: none) are neglecting the render of the opposite side border: let's say, if you have a <thead>, then the bottom border is not rendered, while the layout still reserves the physical space where the border should be (it's just as if the border was transparent).
This only happens when collapse is used, using separate the borders are rendered correctly. In the reduction I created I also added a box-shadow to show how the table still takes its supposed height and that the issue is just with the table border.
Confirmed as a WebKit bug by testing on Safari 5.0.5, 5.1, Chrome 13 (latest stable), Chrome Canary 15.0.874.7 and Android 2.2.2 browser. The test case is correctly rendered by all the other non-WebKit browsers except for IE8, where at first glance same bug can be seen (didn't try much to debug with separate instead of collapse, but IE7 and IE9 render this correctly).
Let me know if you need any more info on this. I saw there are a bunch of issues with border: collapse but couldn't find anything that describes this scenario.
Tested against IE9, Opera and FF and I confirm your findings: WebKit is the only engine not having a bottom border. Great test case and testing, thanks!
Created attachment 141673 [details]
Proposed patch added:
When calculating the collapsed before border, for obtaining the section above the current section, we call sectionAbove(). To this the second param should be passed as SkipEmptySections. Thus for the currSection being the top most section of the table, the returned result would be null which should be the expected output, thus enabling the before table border to be applied for the top most section.
Similarly, when calculating the collapsed after border, we call sectionBelow() for obtaining the section following the currSection. To this call as well we should pass the second param as SkipEmptySections so that for the bottom most section null is returned. This would thus apply the after table border to the bottom most section.
This fix causes an existing layout test case's - fast/table/empty-section-crash.html result to be changed. This is expected with the fix and same result is also observed on FF and Opera.
Modified expected files have been currently included only for the Qt platform.
Comment on attachment 141673 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=141673&action=review
> + * platform/efl/Skipped:
> + * platform/gtk/test_expectations.txt:
> + * platform/mac-wk2/Skipped:
> + * platform/mac/Skipped:
> + * platform/wincairo/Skipped:
> + Skipped fast/table/empty-section-crash.html since the expected result
> + changes with the given fix.
> + * platform/qt/fast/table/empty-section-crash-expected.png:
> + * platform/qt/fast/table/empty-section-crash-expected.txt:
> + Added the new expected results for fast/table/empty-section-crash.html for
> + Qt platform.
I think you should switch this test to use layoutTestController.dumpAsText() instead of skipping it all over the place. Your new test covers the rendering change.
Created attachment 142182 [details]
Have uploaded another patch.
Since the previously failing fast/table/empty-section-crash.html was modified to make its expected output platform independent, we no longer get differing results for this testcase after applying the patch.
fast/table/empty-section-crash.html verifies whether the page crashes or not and not the layout per se. For verifying the layout of table with collapsed borders, a reftest has been included in this patch : fast/css/table-collapsed-borders.html.
Comment on attachment 142182 [details]
Clearing flags on attachment: 142182
Committed r117271: <http://trac.webkit.org/changeset/117271>
All reviewed patches have been landed. Closing bug.