1395650 - Make anonymous colgroups non-inheriting anon boxes

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement)

Description

[Boris Zbarsky [:bzbarsky]]

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".

Comment 1

[Boris Zbarsky [:bzbarsky]]

Actually, the relevant testcase is:

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

Comment 2

[Boris Zbarsky [:bzbarsky]]

And even more actually, it's:

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

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Make anonymous column groups into non-inheriting anon boxes, to better match the behavior of other browsers

Comment 4

[Cameron McCormack (:heycam)]

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?

Comment 5

[Boris Zbarsky [:bzbarsky]]

> 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

[Pulsebot]

Pushed by [e-mail]:
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

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/2cbfa0e50247