This bug was noticed in Safari 3 beta and is also present in WebKit nightly r23682. The problem occurs for a table element with a border that is using the collapsing border model. If the number of columns in the table is determined by a single td element in each row, each with the same colspan attribute of a value greater than 3, then the right border on the table does not show up. For example if the table contents were: <tr><td colspan="3">Cell one</td></tr> <tr><td colspan="3">Cell two</td></tr> Incidently, if colspan is 2 on both those lines instead, then it displays correctly.
Created attachment 15155 [details] Reduced test case This is a reduced test case showing the problem. The table should show a border on four sides instead of just three.
Created attachment 15156 [details] Reduced test case Corrected the test case to match description in original report.
Created attachment 20868 [details] Reduced test case for bugzilla.mozilla.org This bug also affects default Bugzilla styling on <https://bugzilla.mozilla.org/> when rendering the "Attachments" table with no attachments: https://bugzilla.mozilla.org/show_bug.cgi?id=431052
Not a regression as this happens in Safari 2.0.4 on Mac OS X 10.4.11 as well.
See also bug 5515.
This looks pretty bad (on bugs with no attachments) now that bugs.webkit.org has upgraded to a recent Bugzilla.
Created attachment 41987 [details] Reduced test case for border conflict resolution This bug also manifests itself when border conflicts need to be resolved. As shown in the attached reduced test case, the thin border set on <td colspan=3> wins over the thick border set on the enclosing <tr>.
I'm working on this bug now. It seems that there is just incorrect implementation for RenderTable::colToEffCol(). Second problem is that that I don't understand why there is m_columns.fill(ColumnStruct(), 1); in constructor of RenderTable. It causes for example that in this case we have 2 effective columns instead of one. This is BTW reason why this bug is reproducable with colSpan=3, but not with colSpan=2. So, I'm going to remove this extra column from constructor. Unfortunately this breaks several tests, some of which as platform specific. However I don't see any good reason to have these tests actually platform specific and going to rewrite them using Ahem font. Can anyone comment these ideas?
Created attachment 107657 [details] Initial patch, test failures expected
Comment on attachment 107657 [details] Initial patch, test failures expected Attachment 107657 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9735088 New failing tests: platform/chromium/fast/text/text-stroke-with-border.html fast/table/inline-form-assert.html scrollbars/disabled-scrollbar.html fast/invalid/018.html scrollbars/scrollbar-buttons.html fast/invalid/table-residual-style-crash.html fast/invalid/017.html scrollbars/listbox-scrollbar-combinations.html platform/mac-snowleopard/platform/mac/fast/text/international/Geeza-Pro-vertical-metrics-adjustment.html fast/invalid/residual-style.html scrollbars/overflow-scrollbar-combinations.html scrollbars/basic-scrollbar.html fast/invalid/020.xml fast/forms/file-input-disabled.html fast/invalid/table-inside-stray-table-content.html
Created attachment 108228 [details] Probably will pass cr-linux tests
Comment on attachment 108228 [details] Probably will pass cr-linux tests View in context: https://bugs.webkit.org/attachment.cgi?id=108228&action=review > Source/WebCore/rendering/RenderTable.h:164 > + for (unsigned c = 0; effCol < numCols && c + m_columns[effCol].span - 1 < col; ++effCol) What prevents overflow in the "c + span - 1" expression? > Source/WebCore/rendering/RenderTable.h:165 > + c += m_columns[effCol].span; What prevents overflow in this addition? (I know itβs not new.)
Nothing prevents overflow here and in other places. Actually once you set span a little greater than 1 billion (actually >= 2^30) we get overflow somewhere else (may be in appendColumn()) and span value in m_columns becomes invalid. So yes, there is problem. I'd say that all cells locations arithmetic should be done using unsigned just because this seems logical. But even in this case there still place for overflow. I'm not sure how to solve this, except of may be adding some limitation on "span" value and number of columns, like maxColSpan=10000 and maxColCount=100000. Alternative approach is using longer number, or even number with unlimited precision. But I don't think that we should go so far. In any case, this sounds as separate enhancement, unrelated with this bug. Do you want me open new one?
Comment on attachment 108228 [details] Probably will pass cr-linux tests r=me
Comment on attachment 108228 [details] Probably will pass cr-linux tests Clearing flags on attachment: 108228 Committed r96509: <http://trac.webkit.org/changeset/96509>
All reviewed patches have been landed. Closing bug.
Additional baselines updates in http://trac.webkit.org/changeset/96516. Please let me know if any of those are in error.
Yes, these changes are expected, they reflect same changes in Chromium Linux version.
Thanks for checking!