1517067 - Remove "table-caption" special case from dynamic reflow root

Status: RESOLVED FIXED

(Core :: Layout, enhancement, P3)

Description

[Daniel Holbert [:dholbert]]

Right now, InitDynamicReflowRoot has a special case with the broad explanation:
>    // We have a display type where 'width' and 'height' don't actually
>    // set the width or height (i.e., the size depends on content).
...which includes a check for table-caption frames:
> [...] || display == StyleDisplay::TableCaption ||
https://searchfox.org/mozilla-central/rev/bcc0fe1081dc4f84fc5fdfebe0d2de8c075a4e2c/layout/generic/ReflowInput.cpp#825,828-829

However, from some quick testing, it appears to me that table-caption frames *do* respect their width and height properties (when they are set to fixed values).  If you have a caption with e.g.
  height: 5px;
  width:  5px;
  background: red;

...then its orange box is 5px by 5px, even if its content happens to be larger, and even if it happens to be in a table that happens to be larger in one dimension or the other.

So I don't think we need this table-caption exemption after all. Or, if we do, I need help figuring out how to trigger the sort of failure that causes us to need this exemption, for the benefit of the automated test that I'm writing in bug 1508420.

Comment 1

[Daniel Holbert [:dholbert]]

dbaron, it looks like the "table-caption" check here dates back to your first patches in bug 1159042 comment 7. (search for "caption" in https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/22ba869b38a4/set-dynamic-reflow-root )

Do you agree that we can remove this exemption, or is there something I'm not thinking of here?

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: testcase 1 (demonstrating that table-caption honors its width and height)

Comment 3

[Daniel Holbert [:dholbert]]

Note: table-caption does have one kinda-special sizing behavior -- its width[1] (whether specified or content-based) does establish a minimum width for the table itself.

However, that's not really that special; the same is true for *any* element that has an intrinsically-sized ancestor; and that doesn't prevent us from making the inner element a dynamic reflow root (if it satisfies all the conditions, e.g. its own size is fixed).

Assuming the table caption has a specified pixel-valued size which remains fixed (and that it satisfies all the other dynamic reflow root requirements RE being a fixed-position CB etc.), I believe it's valid for us to label it as a dynamic reflow root and to initiate reflows at the caption itself if the only dirty frames are inside of it.


[1] (s/width/height/ depending on the caption side, of course)

Comment 4

[Daniel Holbert [:dholbert]]

(meant to set ni=dbaron (post-vacation) for comment 1)

Comment 5

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

This sounds reasonable to me; I don't particularly remember the rationale here.

I think it's probably worth double-checking two things:
(1) that it's true for all six values of caption-side (top, bottom, left, right, top-outside, bottom-outside)
(2) that it's also true in other browser engines.

If so, I think it seems fine to remove this exception.

Comment 6

[Daniel Holbert [:dholbert]]

(In reply to David Baron from comment #5)

Thanks for the reply.

I think it's probably worth double-checking two things:
(1) that it's true for all six values of caption-side (top, bottom, left, right, top-outside, bottom-outside)

It seems to be. I opened the attached testcase (with a fixed-size caption whose contents & are larger than its fixed-size, and whose table is also larger), and I ran through the caption-side values you listed via tweaks in devtools. And in all cases, the caption was only as large as its specified size (with its too-large-content overflowing).0

(2) that it's also true in other browser engines.

I tried Chrome and Safari, and it's true there for the two caption-side values that they support ("top" and "bottom").

If so, I think it seems fine to remove this exception.

Great, thanks!

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: Bug 1517067: Allow table-caption boxes to be dynamic reflow roots. r?dbaron

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9969ea8e9a58
Allow table-caption boxes to be dynamic reflow roots. r=dbaron

Comment 9

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/9969ea8e9a58