Make anonymous colgroups non-inheriting anon boxes

RESOLVED FIXED in Firefox 57

Status

()

RESOLVED FIXED
a year ago
a year ago

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Tracking

53 Branch
mozilla57
Points:
---

Firefox Tracking Flags

(firefox57 fixed)

Details

Attachments

(1 attachment)

This causes a behavior change in this case:

  <table style="visibility: collapse">
    <tr><td>Is this visible?</td></tr>
  </table>

In Gecko the text is not visible, because the anonymous colgroup inherits the visibility style and collapses.  In every single other browser the text is visible.  Safari and Chrome don't implement "visibility: collapse" on columns/colgroups at all, but Edge does and still shows the text in this case.

I'm going to align our behavior with Edge and incidentally reduce the number of table-column-group styles we need to have hanging around normally.

This patch depends on bug 1395312 to not have bad effects on hit-testing when visibility:hidden tables are involved, since the anonymous colgroup will have "visibility: visible".
Actually, the relevant testcase is:

  <table style="visibility: collapse">
    <tr style="visibility: visible"><td>Is this visible?</td></tr>
  </table>
And even more actually, it's:

  <table style="visibility: collapse">
    <tbody style="visibility: visible">
      <tr><td>Is this visible?</td></tr>
    </tbody>
  </table>
Created attachment 8903404 [details] [diff] [review]
Make anonymous column groups into non-inheriting anon boxes, to better match the behavior of other browsers
Attachment #8903404 - Flags: review?(cam)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Comment on attachment 8903404 [details] [diff] [review]
Make anonymous column groups into non-inheriting anon boxes, to better match the behavior of other browsers

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

::: layout/base/GeckoRestyleManager.cpp
@@ +1027,5 @@
> +    // No need to do anything here for this frame, but we should still reparent
> +    // its descendants, because those may have styles that inherit from the
> +    // parent of this frame (e.g. non-anonymous columns in an anonymous
> +    // colgroup).
> +    ReparentFrameDescendants(aFrame, providerChild);

Can we assert in here that we are reparenting a non-inheriting anonymous box?

::: layout/base/ServoRestyleManager.cpp
@@ +1506,5 @@
> +    // No need to do anything here for this frame, but we should still reparent
> +    // its descendants, because those may have styles that inherit from the
> +    // parent of this frame (e.g. non-anonymous columns in an anonymous
> +    // colgroup).
> +    ReparentFrameDescendants(aFrame, providerChild, aStyleSet);

And in here?
Attachment #8903404 - Flags: review?(cam) → review+
> Can we assert in here that we are reparenting a non-inheriting anonymous box?

We should be able too.  I'll check whether try agrees!

Comment 6

a year ago
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2cbfa0e50247
Make anonymous column groups into non-inheriting anon boxes, to better match the behavior of other browsers.  r=heycam
https://hg.mozilla.org/mozilla-central/rev/2cbfa0e50247
Status: ASSIGNED → RESOLVED
Last Resolved: a year 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.