1137148 - [BC] border-style: groove not applied for tables with border-collapse: collapse

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

[Fernando Alvarez]

Attachment: minimum html to reproduce the problem. Basically a table with a border-collapse: collapse attribute and cells with border-style: groove

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:36.0) Gecko/20100101 Firefox/36.0
Build ID: 20150224134236

Steps to reproduce:

Load the attached file in a desktop browser (does not matter under Windows or Linux, in both cases is reproduceable).


Actual results:

It looks like more than the solid style: "Displays a single, straight, solid line."


Expected results:

According to the attribute description at https://developer.mozilla.org/en-US/docs/Web/CSS/border-style : "Displays a border leading to a carved effect. It is the opposite of ridge."

Comment 1

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

What does a white groove border do in other contexts?

Comment 2

[Boris Zbarsky [:bzbarsky]]

So this is a regression.  This worked correctly in fx35 but is broken in fx36.

In particular, this broke in bug 1083557 as far as I can tell.  It totally changed how the painting happens here and is passing in the wrong border colors.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix painting of ridge/groove borders for border-collapsed tables

I'm not sure how to write a test for this, short of a != reftest (which _would_ have caught the problem, so maybe I should add that)

Comment 4

[Gérard Talbot]

(In reply to Boris Zbarsky [:bz] from comment #3)
> I'm not sure how to write a test for this, short of a != reftest (which
> _would_ have caught the problem, so maybe I should add that)

Would this one help?

http://www.gtalbot.org/BrowserBugsSection/css21testsuite/Bug1137148-border-style-groove-table-cell.html

---------

Just for your information.

The ridge correspondent of this bug is
Bug 1136183 : [BC] ridge border style on border-collapsed table

Comment 5

[Gérard Talbot]

> I'm not sure how to write a test for this, short of a != reftest (which
> _would_ have caught the problem, so maybe I should add that)

http://test.csswg.org/suites/css2.1/latest/html4/border-style-outset-002.htm

was submitted and approved in CSS 2.1 test suite...

although

http://test.csswg.org/suites/css2.1/nightly-unstable/html4/border-style-outset-002.htm

seems better..

Gérard

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Now with tests

Gérard, thanks!  That seems to work pretty well.

Comment 7

[Jonathan Watt [:jwatt]]

Comment on attachment 8634507 [details] [diff] [review]
Now with tests

Thanks for fixing this, and sorry for the lengthy delay digging back through the breaking code.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Jonathan, any idea on those XXX comments about the SetColor calls?

Comment 9

[Jonathan Watt [:jwatt]]

I've not, no. I only removed calls that I could convince myself were unneeded and left the others. If the code is hard to follow in this case I'd be okay with removing them and, if anything breaks, re-adding them.

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/40164bf73534

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/40164bf73534