3964 – Dotted borders render w/ artifacts and sometimes as solid lines

RESOLVED FIXED 3964

Dotted borders render w/ artifacts and sometimes as solid lines

https://bugs.webkit.org/show_bug.cgi?id=3964

Summary Dotted borders render w/ artifacts and sometimes as solid lines

Eric O'Connell

Reported 2005-07-12 10:47:36 PDT

This bug seems to be mainly affected by the length of the th element relative to the td's. Only occurs on tables with border-collapse: collapse & dotted border on th element.

Attachments
Colspan above causes border to extend to other cells (417 bytes, text/html) 2005-12-28 07:42 PST, mitz	no flags	Details
Minimal test case, border-collapse, two rows, no colspan (490 bytes, text/html) 2011-06-27 09:26 PDT, kitchin	no flags	Details
HTML, minimal test case, collapsed and thin dotted borders (419 bytes, text/html) 2013-02-16 20:34 PST, William Rummler	no flags	Details
WIP - 1 (3.72 KB, patch) 2013-03-25 12:49 PDT, Pravin D	no flags	Details Formatted Diff Diff
Archive of layout-test-results from gce-cr-linux-05 for chromium-linux-x86_64 (1.99 MB, application/zip) 2013-03-27 06:21 PDT, WebKit Review Bot	no flags	Details
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 (747.55 KB, application/zip) 2013-03-27 16:43 PDT, Build Bot	no flags	Details
Archive of layout-test-results from webkit-ews-01 for mac-future (944.75 KB, application/zip) 2013-03-28 06:19 PDT, Build Bot	no flags	Details
Archive of layout-test-results from webkit-ews-05 for mac-future (723.60 KB, application/zip) 2013-03-28 06:48 PDT, Build Bot	no flags	Details
Patch (13.11 KB, patch) 2013-06-13 07:26 PDT, Pravin D	no flags	Details Formatted Diff Diff
Patch (13.18 KB, patch) 2013-06-26 02:46 PDT, Pravin D	no flags	Details Formatted Diff Diff
Patch (13.04 KB, patch) 2013-07-08 02:45 PDT, Pravin D	no flags	Details Formatted Diff Diff
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 (498.87 KB, application/zip) 2013-07-08 03:34 PDT, Build Bot	no flags	Details
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

John Sullivan

Comment 1 2005-07-12 12:51:54 PDT

I see the bug on the page listed here, so confirming.

Eric Seidel (no email)

Comment 2 2005-12-27 14:34:03 PST

Seems like a pattern offset bug. It's possible mitz might know this area of the code.

mitz

Comment 3 2005-12-28 07:42:12 PST

Created attachment 5338 [details] Colspan above causes border to extend to other cells This demonstrates what I think is the core issue here: the cell with colspan 2 on the first row adopts as its bottom border the top border of the first cell on the second row.

Joost de Valk (AlthA)

Comment 4 2005-12-29 00:01:26 PST

Added HasReduction. I think this bug is related to bug 3820, in the very least because both seem to be pointing out errors in code relating to colspan.

Robin Whittleton

Comment 5 2011-01-14 08:37:43 PST

Another minimal testcase: data:text/html;charset=utf-8,<!DOCTYPE html><html lang%3D"en"><head><meta charset%3D"UTF-8" /><style>table{ border-collapse: collapse; }td{ border-bottom: 1px dotted red; }</style></head><body><table><tr><td>Col 1</td><td>Col 2</td></tr><tr><td colspan%3D"2">Cell spanning 2 columns.</td></tr></table></body></html> Getting rid of border-collapse:collapse fixes it. Broken in Chrome 8 and Safari 5. Working in Firefox 4b9

kitchin

Comment 6 2011-06-27 09:26:19 PDT

Created attachment 98738 [details] Minimal test case, border-collapse, two rows, no colspan Any EVEN number in the padding will make the 2px dotted border solid. Tested in Google Chrome 12.0.742.100 on WinXP.

kitchin

Comment 7 2011-06-27 09:29:00 PDT

A workaround for HTML authors is to try changing padding by one pixel.

William Rummler

Comment 8 2013-02-16 20:34:49 PST

Created attachment 188751 [details] HTML, minimal test case, collapsed and thin dotted borders My specific encounter: In a table element with collapsed borders and thin dotted cell borders, the border between a spanning cell and every other one of its neighbor cells becomes solid. Reproduced on Win7 x64 with Webkit r131444 (16 Oct 2012), version 537.17. Did not occur for me using non-thin dotted cell borders. Also fails to reproduce at a few intermediate zoom levels. Attaching another minimal test case.

Arpita Bahuguna

Comment 9 2013-03-22 01:07:03 PDT

The first two testcases seem similar to the issue specified in #5515 (https://bugs.webkit.org/show_bug.cgi?id=5515).

Pravin D

Comment 10 2013-03-25 12:49:48 PDT

Created attachment 194905 [details] WIP - 1

Pravin D

Comment 11 2013-03-25 12:53:44 PDT

(In reply to comment #10) > Created an attachment (id=194905) [details] > WIP - 1 > The patch does not work properly for cells with colspan. Also the cell corners where the borders overlap is not proper. Will upload another addressing these issues.

WebKit Review Bot

Comment 12 2013-03-27 06:02:40 PDT

Attachment 194905 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/rendering/RenderTableCell.cpp']" exit_code: 1 Source/WebCore/rendering/RenderTableCell.cpp:1132: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.

WebKit Review Bot

Comment 13 2013-03-27 06:21:42 PDT

Comment on attachment 194905 [details] WIP - 1 Attachment 194905 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17336108 New failing tests: css2.1/20110323/border-conflict-element-001d.htm http/tests/cache/subresource-failover-to-network.html css2.1/t170602-bdr-conflct-w-19-d.html css2.1/t170602-bdr-conflct-w-25-d.html css2.1/t170602-bdr-conflct-w-03-d.html css2.1/20110323/border-conflict-style-079.htm css2.1/t170602-bdr-conflct-w-11-d.html css2.1/t170602-bdr-conflct-w-07-d.html css2.1/t170602-bdr-conflct-w-26-d.html css2.1/t170602-bdr-conflct-w-15-d.html css2.1/t170602-bdr-conflct-w-08-d.html css2.1/t170602-bdr-conflct-w-12-d.html css2.1/t170602-bdr-conflct-w-02-d.html css2.1/t170602-bdr-conflct-w-21-d.html css2.1/t170602-bdr-conflct-w-23-d.html css2.1/20110323/border-collapse-offset-002.htm css2.1/t170602-bdr-conflct-w-17-d.html css2.1/t170602-bdr-conflct-w-28-d.html css2.1/t170602-bdr-conflct-w-06-d.html css2.1/20110323/border-conflict-style-088.htm css2.1/t170602-bdr-conflct-w-16-d.html css2.1/t170602-bdr-conflct-w-24-d.html css2.1/t170602-bdr-conflct-w-13-d.html css2.1/t170602-bdr-conflct-w-01-d.html css2.1/t170602-bdr-conflct-w-04-d.html css2.1/t170602-bdr-conflct-w-18-d.html css2.1/t170602-bdr-conflct-w-27-d.html css2.1/t170602-bdr-conflct-w-14-d.html css2.1/t170602-bdr-conflct-w-05-d.html css2.1/t170602-bdr-conflct-w-22-d.html

WebKit Review Bot

Comment 14 2013-03-27 06:21:46 PDT

Created attachment 195299 [details] Archive of layout-test-results from gce-cr-linux-05 for chromium-linux-x86_64 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-05 Port: chromium-linux-x86_64 Platform: Linux-3.3.8-gcg-201212281604-x86_64-with-GCEL-10.04-gcel_10.04

Build Bot

Comment 15 2013-03-27 16:43:36 PDT

Comment on attachment 194905 [details] WIP - 1 Attachment 194905 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-commit-queue.appspot.com/results/17324194 New failing tests: css2.1/20110323/border-conflict-element-001d.htm

Build Bot

Comment 16 2013-03-27 16:43:39 PDT

Created attachment 195433 [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.2

Build Bot

Comment 17 2013-03-28 06:19:40 PDT

Comment on attachment 194905 [details] WIP - 1 Attachment 194905 [details] did not pass mac-ews (mac): Output: http://webkit-commit-queue.appspot.com/results/17311557 New failing tests: media/video-zoom.html css2.1/20110323/border-conflict-element-001d.htm

Build Bot

Comment 18 2013-03-28 06:19:43 PDT

Created attachment 195550 [details] Archive of layout-test-results from webkit-ews-01 for mac-future The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-01 Port: mac-future Platform: Mac OS X 10.8.2

Build Bot

Comment 19 2013-03-28 06:48:36 PDT

Comment on attachment 194905 [details] WIP - 1 Attachment 194905 [details] did not pass mac-ews (mac): Output: http://webkit-commit-queue.appspot.com/results/17311581 New failing tests: css2.1/20110323/border-conflict-element-001d.htm

Build Bot

Comment 20 2013-03-28 06:48:39 PDT

Created attachment 195559 [details] Archive of layout-test-results from webkit-ews-05 for mac-future The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-05 Port: mac-future Platform: Mac OS X 10.8.2

Pravin D

Comment 21 2013-06-13 07:26:22 PDT

Created attachment 204585 [details] Patch

Elliott Sprehn

Comment 22 2013-06-17 21:46:54 PDT

Comment on attachment 204585 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=204585&action=review > Source/WebCore/rendering/RenderTableCell.cpp:1113 > +bool RenderTableCell::alignTopBottomBorderPaintRect(RenderTableCell* cell, int& topYOffset, int& bottomYOffset) This should just be a method on RenderTableCell instead of passing it as the first argument. It doesn't use any local state. cell->align...(...); > Source/WebCore/rendering/RenderTableCell.cpp:1120 > + bottomYOffset = max<int>(bottomYOffset, (bottom)); extra parens. > Source/WebCore/rendering/RenderTableCell.cpp:1122 > + bShouldDrawBorder = false; return false; > Source/WebCore/rendering/RenderTableCell.cpp:1123 > + return bShouldDrawBorder; return true; > Source/WebCore/rendering/RenderTableCell.cpp:1163 > + int bottomYOffsetLeft = bottomWidth, bottomYOffsetRight = bottomYOffsetLeft; I think we usually do one variable init per line, but I don't know if the style is specific about it. > Source/WebCore/rendering/RenderTableCell.cpp:1171 > + bool shouldDrawLeftBorder = true, shouldDrawRightBorder = true; Same deal. > Source/WebCore/rendering/RenderTableCell.cpp:1174 > + if (top->colSpan() > 1 || this->colSpan() > 1) It's weird that one returns a boolean and one doesn't. > Source/WebCore/rendering/RenderTableCell.cpp:1182 > + shouldDrawLeftBorder = alignTopBottomBorderPaintRect(left, topYOffsetLeft, bottomYOffsetLeft); if (left) left->alignTopBottom...(...);

Pravin D

Comment 23 2013-06-26 02:46:25 PDT

Created attachment 205465 [details] Patch

Elliott Sprehn

Comment 24 2013-07-03 11:07:41 PDT

Comment on attachment 205465 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=205465&action=review Looking at the layout test it's not clear if this is really better, you paint half blocks in the corners now which looks bad. I do see in Safari that if you zoom in the test case the borders will become dashes and sometimes even a solid line. Maybe you want the layout test to adjust the zoom? > Source/WebCore/rendering/RenderTableCell.cpp:1106 > + const RenderStyle* styleForTopCell = this->styleForCellFlow(); this-> is not needed anywhere in here. > Source/WebCore/rendering/RenderTableCell.cpp:1118 > + const RenderStyle* styleForBottomCell = this->styleForCellFlow(); Ditto. > Source/WebCore/rendering/RenderTableCell.cpp:1166 > + int topYOffsetLeft = topWidth; These declarations are a bit weird, why is topWidth the same as topYOffsetLeft ? > Source/WebCore/rendering/RenderTableCell.cpp:1186 > + if (bottom) I'd just merge all these declarations: if (RenderTableCell* bottom = table()->cellBelow(this)) bottom->... > LayoutTests/fast/table/border-collapsing/dotted-collapsed-border.html:1 > +<html> Add a doctype unless you need quirks mode.

Pravin D

Comment 25 2013-07-08 02:45:30 PDT

Created attachment 206225 [details] Patch

Build Bot

Comment 26 2013-07-08 03:34:31 PDT

Comment on attachment 206225 [details] Patch Attachment 206225 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/949125 New failing tests: fullscreen/full-screen-iframe-with-max-width-height.html

Build Bot

Comment 27 2013-07-08 03:34:34 PDT

Created attachment 206228 [details] Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-09 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.3

Pravin D

Comment 28 2013-07-09 00:15:27 PDT

(In reply to comment #24) > (From update of attachment 205465 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=205465&action=review > > Looking at the layout test it's not clear if this is really better, you paint half blocks in the corners now which looks bad. I do see in Safari that if you zoom in the test case the borders will become dashes and sometimes even a solid line. Maybe you want the layout test to adjust the zoom? > Let me explain a little abt the issue.. Lets take the layout testcase as the example. The collapse border(with the issue) are drawn as follows: 1) The top border of the lower cell is drawn. The start of the border is from the left extreme i.e from the left most point till which the left border extends. This translates to start X(top border)S1 = current cell start X - CellLeftWidth/2; end X (top border)E1 = current cell end X - CellRightWidth/2; 2) The bottom border of the top cell is drawn to overlap the previously drawn border. The start and end points are same as above. Lets name the start and end points as S2 and E2. The problem occurs when S1 != S2 and/or E1 != E2. The problem becomes more and more visible when zoomed. The fix(only for dotted border case) is that we try to keep S1 = S2 and E1 = E2 by considering the left and right width of both the top and bottom cells which share the collapsed border. > > Source/WebCore/rendering/RenderTableCell.cpp:1106 > > + const RenderStyle* styleForTopCell = this->styleForCellFlow(); > > this-> is not needed anywhere in here. > > > Source/WebCore/rendering/RenderTableCell.cpp:1118 > > + const RenderStyle* styleForBottomCell = this->styleForCellFlow(); > > Ditto. > done. > > Source/WebCore/rendering/RenderTableCell.cpp:1166 > > + int topYOffsetLeft = topWidth; > > These declarations are a bit weird, why is topWidth the same as topYOffsetLeft ? > Wen drawing the left border the starts from top most point where top border of the cell is drawn. > > Source/WebCore/rendering/RenderTableCell.cpp:1186 > > + if (bottom) > > I'd just merge all these declarations: > > if (RenderTableCell* bottom = table()->cellBelow(this)) > bottom->... > Done > > LayoutTests/fast/table/border-collapsing/dotted-collapsed-border.html:1 > > +<html> > > Add a doctype unless you need quirks mode. > Done

WebKit Commit Bot

Comment 29 2013-07-30 21:58:37 PDT

Comment on attachment 206225 [details] Patch Clearing flags on attachment: 206225 Committed r153510: <http://trac.webkit.org/changeset/153510>

WebKit Commit Bot

Comment 30 2013-07-30 21:58:42 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version 412

Hardware Mac

OS OS X 10.4

Product WebKit

Component Tables

Assignee

Pravin D

Reported

2005-07-12 10:47 PDT

Modified

2013-07-30 21:58 PDT History

CC List

18 users Show

URL

http://www.bodytalksystem.com/test/dotted-borders.html

Keywords HasReduction

Depends on

Blocks