Closed Bug 1688690 Opened 9 months ago Closed 9 months ago

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

Categories

(Core :: Layout: Tables, defect)

defect

Tracking

()

RESOLVED FIXED
87 Branch
Tracking Status
firefox87 --- fixed

People

(Reporter: TYLin, Assigned: TYLin)

References

Details

Attachments

(3 files)

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

Summary: Resolve the dependency on whether the computed margin of nsTableFrame is all zero → Resolve the discrepency on whether the computed margin of nsTableFrame is all zero

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

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

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

Depends on D103065

Pushed by aethanyc@gmail.com:
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
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → 87 Branch
Duplicate of this bug: 1667850
You need to log in before you can comment on or make changes to this bug.