When WebKit renders a table for which both border-collapse: collapse; and overflow: hidden; are set via CSS the rendering of my tables with 1px border looked strange: the bottom and right border were not displayed at all. After some investigation I found that these borders do get rendered somehow, but in another way than usual; this effect can be seen by enlarging the border-width. Please check the website or the attached test case reduction file to see an example of both cases
Created attachment 20323 [details] Test case reduction file (HTML): Border drawing issues
Created attachment 20324 [details] Screenshots of both Firefox and WebKit rendering the tables
Confirmed with Safari 3.0.4 (523.12.2) on Mac OS X 10.4.11 (8S165).
I can also reproduce this bug with the following browsers: Mozilla/5.0 (Windows; U; Windows NT 5.1; pl-PL) AppleWebKit/531.9 (KHTML, like Gecko) Version/4.0.3 Safari/531.9.1 Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US) AppleWebKit/532.0 (KHTML, like Gecko) Chrome/3.0.195.33 Safari/532.0
Created attachment 203510 [details] Patch
Comment on attachment 203510 [details] Patch Attachment 203510 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/668255 New failing tests: css2.1/20110323/overflow-applies-to-014.htm tables/mozilla_expected_failures/marvin/table_overflow_hidden_table.html tables/mozilla_expected_failures/marvin/table_overflow_caption_hidden_table.html tables/mozilla/bugs/bug44505.html css2.1/20110323/overflow-applies-to-013.htm tables/mozilla_expected_failures/bugs/bug106966.html
Created attachment 203513 [details] Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-14 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.3
Comment on attachment 203510 [details] Patch Attachment 203510 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/656645 New failing tests: css2.1/20110323/overflow-applies-to-014.htm tables/mozilla_expected_failures/marvin/table_overflow_hidden_table.html tables/mozilla_expected_failures/marvin/table_overflow_caption_hidden_table.html tables/mozilla/bugs/bug44505.html css2.1/20110323/overflow-applies-to-013.htm tables/mozilla_expected_failures/bugs/bug106966.html
Created attachment 203514 [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.3
Comment on attachment 203510 [details] Patch Attachment 203510 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/708246 New failing tests: css2.1/20110323/overflow-applies-to-014.htm tables/mozilla_expected_failures/marvin/table_overflow_hidden_table.html tables/mozilla_expected_failures/marvin/table_overflow_caption_hidden_table.html tables/mozilla/bugs/bug44505.html css2.1/20110323/overflow-applies-to-013.htm tables/mozilla_expected_failures/bugs/bug106966.html
Created attachment 203515 [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.3
Created attachment 203523 [details] Patch
Comment on attachment 203523 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=203523&action=review r- > Source/WebCore/rendering/RenderTable.cpp:1363 > + if (phase == PaintPhaseChildBlockBackgrounds) { > + rect = borderBoxRectInRegion(region); > + rect.setLocation(location + rect.location()); > + clipScrollBars(rect, relevancy); This code doesn't make much sense to me. Isn't the clipScrollbars rectangle going to clip scrollbars in the wrong place?
Created attachment 204187 [details] Patch
Comment on attachment 204187 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=204187&action=review r- > Source/WebCore/rendering/RenderTable.cpp:1364 > + LayoutRect rect; > + if (phase == PaintPhaseChildBlockBackgrounds) { > + rect = borderBoxRectInRegion(region); > + rect.setLocation(location + rect.location()); > + } else > + rect = RenderBox::overflowClipRect(location, region, relevancy); I just don't think you can make this phase-dependent. Make the table into a self-painting layer, e.g., by making it position:absolute, and suddenly the overflowClipRect is going to get cached by the RenderLayer clipRects code, and it's not going to use the phase you want. Basically overflowClipRect can't be giving different answers based off phases.
Created attachment 204533 [details] Patch
Comment on attachment 204533 [details] Patch Attachment 204533 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/869038 New failing tests: fast/table/table-overflow.html fast/repaint/table-cell-collapsed-border-scroll.html
Created attachment 204538 [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.3
Comment on attachment 204533 [details] Patch Attachment 204533 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/869053 New failing tests: fast/table/table-overflow.html fast/repaint/table-cell-collapsed-border-scroll.html
Created attachment 204542 [details] Archive of layout-test-results from webkit-ews-03 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-03 Port: mac-mountainlion Platform: Mac OS X 10.8.3
Created attachment 205318 [details] Patch
Comment on attachment 205318 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=205318&action=review r- > Source/WebCore/rendering/RenderTable.cpp:672 > + info.context->restore(); > + info.context->save(); This code seems to be making the assumption that the last save() you did pushed only the overflow clip? That seems like a bogus assumption.
Created attachment 205934 [details] Patch
Created attachment 206023 [details] Patch
Comment on attachment 206023 [details] Patch Attachment 206023 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1015808 New failing tests: fast/table/overflowHidden.html
Created attachment 206027 [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.3
Comment on attachment 206023 [details] Patch Attachment 206023 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/1019548 New failing tests: fast/table/overflowHidden.html
Created attachment 206041 [details] Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-15 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.3
Comment on attachment 206023 [details] Patch Attachment 206023 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/975044 New failing tests: fast/table/overflowHidden.html
Created attachment 206154 [details] Archive of layout-test-results from APPLE-EWS-4 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: APPLE-EWS-4 Port: win-future Platform: CYGWIN_NT-6.1-WOW64-1.7.20-0.266-5-3-i686-32bit
Created attachment 206601 [details] Patch
Comment on attachment 206601 [details] Patch Attachment 206601 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/1066261
Comment on attachment 206601 [details] Patch Attachment 206601 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/1070023
Comment on attachment 206601 [details] Patch Attachment 206601 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1063339
Comment on attachment 206601 [details] Patch Attachment 206601 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/1069026
Created attachment 206603 [details] Patch
Comment on attachment 206603 [details] Patch Attachment 206603 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/1054576 New failing tests: fast/table/overflowHidden.html
Created attachment 206605 [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.3
Comment on attachment 206603 [details] Patch Attachment 206603 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1069057 New failing tests: fast/table/overflowHidden.html
Created attachment 206606 [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.3
Comment on attachment 206603 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=206603&action=review r- > Source/WebCore/rendering/RenderTableCell.cpp:1004 > +LayoutRect RenderTableCell::overflowClipRect(const LayoutPoint& location, RenderRegion* region, OverlayScrollbarSizeRelevancy relevancy) I don't understand any of these table cell changes really. In the self-painting layers case, the cell gets clipped properly already by overflowClipRectForChildLayers. In the case where the table cell has no layer, why not just simulate the same thing and re-use overflowClipRectForChildLayers. Make the table code actually push the overflowClipRectForChildLayers clip before painting kids. I really think done right you don't have to touch RenderTableCell at all.
Created attachment 207122 [details] Patch
Comment on attachment 207122 [details] Patch r=me. Look at how nice that patch ended up being!
Comment on attachment 207122 [details] Patch Clearing flags on attachment: 207122 Committed r153089: <http://trac.webkit.org/changeset/153089>
All reviewed patches have been landed. Closing bug.