1667501 - Ensure elements not supporting aspect-ratio property are ignoring aspect-ratio

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: table-with-aspect-ratio.html

Per spec, aspect-ratio applies to all elements except inline boxes and internal ruby or table boxes. https://drafts.csswg.org/css-sizing-4/#aspect-ratio

Currently, we do apply aspect-ratio on <table> elements. See the attached test case.

I think the root cause probably is we unconditionally consider aspect-ratio in nsIFrame::ComputeSize(). After bug 1660122, we can call nsIFrame::GetApsectRatio() and get a ratio if the frame supports aspect-ratio.

Comment 1

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Per discussion with emilio on Matrix, it's correct to apply aspect-ratio property on <table>.

We currently unconditionally consider aspect-ratio property in nsIFrame::ComputeSize(). If the size of the elements not supporting aspect-ratio (inline boxes and internal ruby or table boxes) are not computed via the nsIFrame::ComputeSize(), then we're fine to leave it as is (though using GetAspectRatio() added in bug 1660122 may be a little bit ergonomic).

We also need testcases to ensure that those elements not supporting aspect-ratio are covered. Currently, I don't see any in testing/web-platform/tests/css/css-sizing/aspect-ratio.

Comment 2

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 9177999 [details]
table-with-aspect-ratio.html

Obsolete the wrong testcase.

Comment 3

[Boris Chiou [:boris]]

Attachment: Bug 1667501 - Add an nsIFrame::IsFrameOfType bit to say whether frames support aspect-ratio.

aspect-ratio applies to all elements except inline boxes and internal ruby or
table boxes. So we should return false for inline frames, table internal frames
(i.e. except nsTableFrame and nsTableWrapperFrame), and ruby internal frames.

Note: it seems nsRubyBaseContainerFrame and nsRubyTextContainerFrame are
inline, so we return false for them.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bf6cf886b968
Add an nsIFrame::IsFrameOfType bit to say whether frames support aspect-ratio. r=TYLin

Comment 5

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/bf6cf886b968