1688690 - Resolve the discrepency on whether the computed margin of nsTableFrame is all zero

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

On the nsTableWrapperFrame, the margin property is inherited from nsTableFrame [1], and the margin should only have an effect on nsTableWrapperFrame. As a result, some of layout code assume the inner table frame (nsTableFrame) should have zero computed margin such as in SizeComputationInput::InitOffsets [2] and in nsTableWrapperFrame::InitChildReflowInput [3].

However, nsTableFrame is currently still don't have zero computed margin as some of the code claims, as we still compute the margin for nsTableFrame in ReflowInput::CalculateBlockSideMargins [4]. And the nsTableFrame's margin seems to play some roles during the reflow when the table has constrained available block-size [5], and when we compute the final block-size for nsTableWrapperFrame [6]

I feel it makes sense to make the computed margin zero when initializing nsTableFrame's ReflowInput. We should make sure we don't go into [4] for nsTableFrame, and adjust nsTableWrapperFrame's reflow code to acknowledge that.

[1] https://searchfox.org/mozilla-central/rev/851bbbd9d9a38c2785a24c13b6412751be8d3253/layout/style/res/ua.css#30
[2] https://searchfox.org/mozilla-central/rev/851bbbd9d9a38c2785a24c13b6412751be8d3253/layout/generic/ReflowInput.cpp#2519-2521
[3] https://searchfox.org/mozilla-central/rev/851bbbd9d9a38c2785a24c13b6412751be8d3253/layout/tables/nsTableWrapperFrame.cpp#241-249
[4] https://searchfox.org/mozilla-central/rev/851bbbd9d9a38c2785a24c13b6412751be8d3253/layout/generic/ReflowInput.cpp#2382
[5] https://searchfox.org/mozilla-central/rev/851bbbd9d9a38c2785a24c13b6412751be8d3253/layout/tables/nsTableWrapperFrame.cpp#713-724
[6] https://searchfox.org/mozilla-central/rev/851bbbd9d9a38c2785a24c13b6412751be8d3253/layout/tables/nsTableWrapperFrame.cpp#926,939-941

Comment 1

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1688690 Part 1 - Extract content-box inline-size in nsTableWrapperFrame::Reflow().

aOuterRI.ComputedSize(wm).ISize(wm) can be extracted as a variable
because it is used repeatedly.

Comment 2

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1688690 Part 2 - Ensure nsTableFrame's computed margins are all zero.

This patch makes the following changes:

1. Don't call ReflowInput::CalculateBlockSideMargins() for nsTableFrame
   so that setting nsTableFrame's computed margins to zero in
   SizeComputationInput::InitOffsets() remains true. Also, add an assertion
   in nsTableFrame::Reflow() to ensure that.

2. Remove useless nsTableFrameWrapper::GetChildMargin() because the
   method is used to get nsTableFrame's margins, which are now all zero.
   Also, the old code that subtracts the block-axis margin from available
   block-size doesn't really make sense.

3. Pass all-zero innerMargins to nsTableWrapperFrame::SetDesiredSize(),
   and use table wrapper's content-box inline-size as the final desired
   border-box inline-size rather than reconstructing it from caption and
   inner table's inline-size & margin like the old code.

This inline-size already takes inner table's intrinsic size and
caption's inline-size into consideration in
nsTableWrapperFrame::ComputeAutoSize(), and is the final inline-size we
want to use.

In the next Part, we are going to simplify all nsTableWrapperFrame's
methods that take inner frame's margin.

Depends on D103064

Comment 3

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1688690 Part 3 - Simplify nsTableWrapperFrame's methods where the nsTableFrame's margin is used.

Rename SetDesiredSize() to ComputeFinalBSize() because it now computes
only the final block-size.

Depends on D103065

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a97f53ce7197
Part 1 - Extract content-box inline-size in nsTableWrapperFrame::Reflow(). r=emilio
https://hg.mozilla.org/integration/autoland/rev/2618795e31c0
Part 2 - Ensure nsTableFrame's computed margins are all zero. r=emilio
https://hg.mozilla.org/integration/autoland/rev/8e011d62b5d8
Part 3 - Simplify nsTableWrapperFrame's methods where the nsTableFrame's margin is used. r=emilio

Comment 5

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/a97f53ce7197
https://hg.mozilla.org/mozilla-central/rev/2618795e31c0
https://hg.mozilla.org/mozilla-central/rev/8e011d62b5d8