654167 - 1px double borders not painting at all with border-collapse:collapse

Status: VERIFIED FIXED

(Core :: Layout: Tables, defect, P2)

Description

[Paul Grosso]

User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:2.0.1) Gecko/20100101 Firefox/4.0.1
Build Identifier: Firefox 4.0.1 ("up to date" as of 2011 May 2)

Using the collapsing border model in tables, I am not always getting the border conflict resolution behavior I expect.  The page at http://paulgrosso.name/firefox/tablerules.htm is self-documenting and contains more details.

Reproducible: Always


Actual Results:  
See http://paulgrosso.name/firefox/tablerules.htm

Expected Results:  
See http://paulgrosso.name/firefox/tablerules.htm

Comment 1

[Boris Zbarsky [:bzbarsky]]

So the issue here is that in the collapsed border model a 1px wide "double" border is not showing up, right?  That probably has to do with the fact that painting a 1.33px wide "double" (which really needs at least 3px to show up) border  _exactly_ on top of a pixel boundary doesn't work very well, not with border conflict resolution...  In the separated border case, the drawing code is different, and the border is positioned on top of a pixel, not on top of a boundary, so things work out better.

We should probably just treat "double" borders thinner than 3px as solid in the table BC drawing code.

Comment 2

[Paul Grosso]

Perhaps that is the crux of the issue, because when I made its style solid (or dashed, etc.), it does show.  In which case it isn't a conflict resolution issue.  I was misled when the separated border case showed the 1pt double rule properly (I was not aware that the drawing code is different in that case).  Thanks for the response.

Comment 3

[Paul Grosso]

But now compare http://paulgrosso.name/firefox/ff4-border-collapse.htm and http://paulgrosso.name/firefox/ff4-border-collapse2.htm which are identical except the second has an extra global CSS rule setting border style to none for the top and left rules on hd and th.  In the first case, the top rules for the third and fourth table are black, not red as I would have expected.  It looks like the default th, td border style is 1.5pt, black, solid, so that the default is taking precedence over a specified solid rules unless what's specified is at least 1.5pt wide.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> It looks like the default th, td border style is 1.5pt, black, solid

The default styles if you specify rules="none" are the ones in bug 648531 comment 2.  Those include "border-width: thin", which should be 1px wide.  The border-collapse code converts all widths into integer pixel widths before doing conflict resolution (which is arguably a bug) and it looks like it does so by truncating, not rounding (another bug).  1pt is 1.333333px.  So 2px is 1.5pt, and hence any value smaller than 1.5pt will be truncated to 1px.  And then the thin border is the same width and it wins because of rule 4 in border conflict resolution.

Worth checking whether the bugs about comparing after conversion to integer pixels and the fact that the conversion truncates instead of rounding are filed; if not, we should get them on file.  But we should definitely fix the painting issue.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Small testcase for that problem

Comment 6

[Boris Zbarsky [:bzbarsky]]

Here's the problem in nsCSSRendering::DrawTableBorderSegment :

  case NS_STYLE_BORDER_STYLE_DOUBLE:
    if ((aBorder.width > 2) && (aBorder.height > 2)) {

Except aBorder is in app units (so 1px is 60, not 1).

Fix coming up once I figure out when this broke.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Looks like this has been broken at least back to Firefox 3.0.  Gotta love lack of tests...

Comment 8

[Boris Zbarsky [:bzbarsky]]

And broken in Firefox 2 as well.  I'm going to stop worrying about this!

Comment 9

[Boris Zbarsky [:bzbarsky]]

This might just be broken back to bug 41262 or earlier.  Anyway, patch coming up.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix drawing of thin 'border-style: double' border-collapsed borders.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Oops.  I should have searched for existing bugs.  :(

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 529563 [details] [diff] [review]
Fix drawing of thin 'border-style: double' border-collapsed borders.

Review of attachment 529563 [details] [diff] [review]:

Comment 14

[Boris Zbarsky [:bzbarsky]]

http://hg.mozilla.org/mozilla-central/rev/798050382d9a

Comment 15

[Simona Badau, Desktop QA]

Mozilla/5.0 (Windows NT 5.1; rv:6.0) Gecko/20100101 Firefox/6.0

Verified issue on Windows XP, Windows 7, Mac OS X 10.6 and Ubuntu using the page from Comment 0 (http://paulgrosso.name/firefox/tablerules.htm) and the test case attached in Comment 5.

Setting resolution to VERIFIED FIXED.