Our current border collapsing code wrongly ignores any direction queues below the table-row-group (section in WebKit speak). Other browsers (and the specification) don't have this limitation. This is another step towards fixing bug 79272 and properly supporting mixed direction at the cell level.
Created attachment 172074 [details] Proposed change: fixed the collapsing border code.
Comment on attachment 172074 [details] Proposed change: fixed the collapsing border code. View in context: https://bugs.webkit.org/attachment.cgi?id=172074&action=review r- for the test. > Source/WebCore/ChangeLog:43 > + Updated these functios to work with mixed directionality. typo: functios > Source/WebCore/ChangeLog:56 > + This is a bug in our implementation: we used to return a non-zero width for inexistant borders (per CSS 2.1, > + 'border-style: off | hidden' should have a 0 width). I don't see a test for this fix. > Source/WebCore/rendering/RenderTableSection.cpp:1276 > + ASSERT_UNUSED(cell, cell->isFirstOrLastCellInRow()); this should just be an ASSERT now. > LayoutTests/fast/table/border-collapsing/table-ltr-rows-mixed-direction-expected.html:1 > +<!DOCTYPE> This doctype is quirksmode I believe. You should be using <!DOCTYPE html>.
Comment on attachment 172074 [details] Proposed change: fixed the collapsing border code. View in context: https://bugs.webkit.org/attachment.cgi?id=172074&action=review >> Source/WebCore/ChangeLog:56 >> + 'border-style: off | hidden' should have a 0 width). > > I don't see a test for this fix. This is covered by LayoutTests/fast/table/border-collapsing/last-cell-left-border-hidden-table-ltr-section-rtl.html which was showing a diff due to having a 3px hidden border returning a width of 3px instead of 0px. >> LayoutTests/fast/table/border-collapsing/table-ltr-rows-mixed-direction-expected.html:1 >> +<!DOCTYPE> > > This doctype is quirksmode I believe. You should be using <!DOCTYPE html>. Good catch, dumb error. This should put the document in quirksmode per HTML5 parsing.
Comment on attachment 172074 [details] Proposed change: fixed the collapsing border code. View in context: https://bugs.webkit.org/attachment.cgi?id=172074&action=review >>> Source/WebCore/ChangeLog:56 >>> + 'border-style: off | hidden' should have a 0 width). >> >> I don't see a test for this fix. > > This is covered by LayoutTests/fast/table/border-collapsing/last-cell-left-border-hidden-table-ltr-section-rtl.html which was showing a diff due to having a 3px hidden border returning a width of 3px instead of 0px. I don't see a diff for this in this patch. Maybe you forgot to include it?
Comment on attachment 172074 [details] Proposed change: fixed the collapsing border code. View in context: https://bugs.webkit.org/attachment.cgi?id=172074&action=review >>>> Source/WebCore/ChangeLog:56 >>>> + 'border-style: off | hidden' should have a 0 width). >>> >>> I don't see a test for this fix. >> >> This is covered by LayoutTests/fast/table/border-collapsing/last-cell-left-border-hidden-table-ltr-section-rtl.html which was showing a diff due to having a 3px hidden border returning a width of 3px instead of 0px. > > I don't see a diff for this in this patch. Maybe you forgot to include it? The test was showing a local diff before I touched CollapsedBorderValue::width. The diff was wrong and was what caused me to find this bug (ie that we could have a 3px hidden green border that would add 3px to our cell's logical width). The root cause of this change is that we are changing which borders are considered during resolution. FWIW our collapsing border resolution code is not very air-tight due to its complexity and this should make it more robust.
Created attachment 172089 [details] Updated change: fixed per Ojan's comment.
Comment on attachment 172089 [details] Updated change: fixed per Ojan's comment. Clearing flags on attachment: 172089 Committed r133439: <http://trac.webkit.org/changeset/133439>
All reviewed patches have been landed. Closing bug.