Closed Bug 1395650 Opened 2 years ago Closed 2 years ago

Make anonymous colgroups non-inheriting anon boxes

Categories

(Core :: CSS Parsing and Computation, enhancement)

53 Branch
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(1 file)

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>
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!
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
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.