Table backgrounds should use the visibility of the cell, not of the col/colgroup/row/rowgroup

RESOLVED FIXED in Firefox 57

Status

()

Core
Layout: Tables
RESOLVED FIXED
3 months ago
2 months ago

People

(Reporter: bz, Assigned: bz)

Tracking

53 Branch
mozilla57
Points:
---

Firefox Tracking Flags

(firefox57 fixed)

Details

Attachments

(2 attachments)

Per spec at https://drafts.csswg.org/css-tables-3/#drawing-cell-backgrounds cell backgrounds (including ones arising from rows/cols) are supposed to only be drawn only on visible cells.  That means they're not drawn on cells with "visibility:hidden" or on empty cells when "empty-cells: hide" is in play.

The attached testcase shows that Chrome, Safari, and Edge all get this right but we do not.

The good thing about doing this is that it will allow us to remove the hackery for considering colgroup visibility that I added in bug 1394662, because we would just consider the cell visibility to start with when deciding whether to generate the background bits.
Er, I forgot to attach the testcase, but the tests in the patch should make it all clear.  ;)
Created attachment 8902997 [details] [diff] [review]
part 1.  Add a method on nsTableCellFrame to determine whether it should paint borders and backgrounds
Attachment #8902997 - Flags: review?(cam)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Created attachment 8902998 [details] [diff] [review]
part 2.  Table backgrounds should be drawn or not based on the visibility of the relevant cell, not of the column/row/colgroup/rowgroup they come from
Attachment #8902998 - Flags: review?(cam)
Attachment #8902997 - Flags: review?(cam) → review+
Comment on attachment 8902998 [details] [diff] [review]
part 2.  Table backgrounds should be drawn or not based on the visibility of the relevant cell, not of the column/row/colgroup/rowgroup they come from

Review of attachment 8902998 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/reftests/table-background/reftest.list
@@ +62,5 @@
>  == table-row-opacity-dynamic-2.html table-row-opacity-dynamic-2-ref.html
> +
> +== hidden-cells-1.html about:blank
> +== hidden-cells-2.html about:blank
> +== hidden-cells-3.html hidden-cells-3-ref.html 

Nit: trailing space.
Attachment #8902998 - Flags: review?(cam) → review+

Comment 5

3 months ago
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/45f2e00b9934
part 1.  Add a method on nsTableCellFrame to determine whether it should paint borders and backgrounds.  r=heycam
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f243b19cba1
part 2.  Table backgrounds should be drawn or not based on the visibility of the relevant cell, not of the column/row/colgroup/rowgroup they come from.  r=heycam
Blocks: 1395650

Comment 6

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/45f2e00b9934
https://hg.mozilla.org/mozilla-central/rev/3f243b19cba1
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.