Skip to content

Commit

Permalink
Bug 1350037 Part 3 - Prevent table caption from honoring justify-*/al…
Browse files Browse the repository at this point in the history
…ign-* when table is a grid item. r=dholbert

The old code used to apply ShrinkWrap flag to both the inner table and the
caption if the table wrapper is a stretched grid item. However, per analysis in
bug 1350037 comment 4, when we reflow table captions, we'd like it *not* to
shrink-wrap, but to stretch its inline-size to at least as wide as the inner
table frame [1].

This patch moves the logic that computes whether a table is a stretched grid
item from `ReflowInput` to `nsTableWrapperFrame`, and apply the `ShrinkWrap`
flag to inner table if needed.

[1] https://searchfox.org/mozilla-central/rev/54d6d78c3b33c92fd9eef9c0555a153bf8e4f2b4/layout/tables/nsTableWrapperFrame.cpp#760-763

Differential Revision: https://phabricator.services.mozilla.com/D196705
  • Loading branch information
aethanyc committed Dec 18, 2023
1 parent d9e2309 commit 05409a1
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 31 deletions.
15 changes: 1 addition & 14 deletions layout/generic/ReflowInput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2371,20 +2371,7 @@ void ReflowInput::InitConstraints(
alignCB = alignCBParent;
}
}
if (alignCB->IsGridContainerFrame()) {
// Shrink-wrap grid items that will be aligned (rather than stretched)
// in its inline axis.
auto inlineAxisAlignment =
wm.IsOrthogonalTo(cbwm)
? mStylePosition->UsedAlignSelf(alignCB->Style())._0
: mStylePosition->UsedJustifySelf(alignCB->Style())._0;
if ((inlineAxisAlignment != StyleAlignFlags::STRETCH &&
inlineAxisAlignment != StyleAlignFlags::NORMAL) ||
mStyleMargin->mMargin.GetIStart(wm).IsAuto() ||
mStyleMargin->mMargin.GetIEnd(wm).IsAuto()) {
mComputeSizeFlags += ComputeSizeFlag::ShrinkWrap;
}
} else {
if (!alignCB->IsGridContainerFrame()) {
// Shrink-wrap blocks that are orthogonal to their container.
if (isBlockLevel && mCBReflowInput &&
mCBReflowInput->GetWritingMode().IsOrthogonalTo(mWritingMode)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
background: lightgrey;
}
caption { border: 1px dashed blue; }
caption { justify-self: stretch; } /* XXX: Workaround bug 1350037. */
x { display:block; width:16px; height:16px; }
t { display:block; width:6px; height:6px; }

Expand Down
34 changes: 18 additions & 16 deletions layout/tables/nsTableWrapperFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "mozilla/ComputedStyle.h"
#include "mozilla/PresShell.h"
#include "nsFrameManager.h"
#include "nsGridContainerFrame.h"
#include "nsTableFrame.h"
#include "nsTableCellFrame.h"
#include "nsStyleConsts.h"
Expand Down Expand Up @@ -430,20 +431,7 @@ LogicalSize nsTableWrapperFrame::ComputeAutoSize(
// actual size of the table, which (if it is specified as a percent)
// could be something that is not reflected in our GetMinISize and
// GetPrefISize. See bug 349457 for an example.

// Shrink-wrap aChildFrame by default, except if we're a stretched grid item.
ComputeSizeFlags flags(ComputeSizeFlag::ShrinkWrap);
if (MOZ_UNLIKELY(IsGridItem()) && !StyleMargin()->HasInlineAxisAuto(aWM)) {
const auto* parent = GetParent();
auto inlineAxisAlignment =
aWM.IsOrthogonalTo(parent->GetWritingMode())
? StylePosition()->UsedAlignSelf(parent->Style())._0
: StylePosition()->UsedJustifySelf(parent->Style())._0;
if (inlineAxisAlignment == StyleAlignFlags::NORMAL ||
inlineAxisAlignment == StyleAlignFlags::STRETCH) {
flags -= ComputeSizeFlag::ShrinkWrap;
}
}
const ComputeSizeFlags flags = CreateComputeSizeFlagsForChild();

// Match the logic in Reflow() that sets aside space for the caption.
Maybe<StyleCaptionSide> captionSide = GetCaptionSide();
Expand Down Expand Up @@ -555,6 +543,17 @@ void nsTableWrapperFrame::GetInnerOrigin(const MaybeCaptionSide& aCaptionSide,
}
}

ComputeSizeFlags nsTableWrapperFrame::CreateComputeSizeFlagsForChild() const {
// Shrink-wrap child frames by default, except if we're a stretched grid item.
if (MOZ_UNLIKELY(IsGridItem())) {
auto* gridContainer = static_cast<nsGridContainerFrame*>(GetParent());
if (gridContainer->GridItemShouldStretch(this, eLogicalAxisInline)) {
return {};
}
}
return {ComputeSizeFlag::ShrinkWrap};
}

void nsTableWrapperFrame::CreateReflowInputForInnerTable(
nsPresContext* aPresContext, nsTableFrame* aTableFrame,
const ReflowInput& aOuterRI, Maybe<ReflowInput>& aChildRI,
Expand Down Expand Up @@ -588,11 +587,13 @@ void nsTableWrapperFrame::CreateReflowInputForInnerTable(
}
}

ComputeSizeFlags csFlags = CreateComputeSizeFlagsForChild();
if (!aTableFrame->IsBorderCollapse() &&
!aOuterRI.mStyleSizeOverrides.HasAnyOverrides()) {
// We are not border-collapsed and not given any size overrides. It's
// sufficient to call the standard ReflowInput constructor.
aChildRI.emplace(aPresContext, aOuterRI, aTableFrame, availSize, cbSize);
aChildRI.emplace(aPresContext, aOuterRI, aTableFrame, availSize, cbSize,
ReflowInput::InitFlags{}, StyleSizeOverrides{}, csFlags);
return;
}

Expand All @@ -618,7 +619,8 @@ void nsTableWrapperFrame::CreateReflowInputForInnerTable(
aBSizeOccupiedByCaption);

aChildRI.emplace(aPresContext, aOuterRI, aTableFrame, availSize, Nothing(),
ReflowInput::InitFlag::CallerWillInit, innerOverrides);
ReflowInput::InitFlag::CallerWillInit, innerOverrides,
csFlags);
aChildRI->Init(aPresContext, cbSize, Some(*borderPadding - *padding),
padding);
}
Expand Down
10 changes: 10 additions & 0 deletions layout/tables/nsTableWrapperFrame.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#ifndef nsTableWrapperFrame_h__
#define nsTableWrapperFrame_h__

#include "LayoutConstants.h"
#include "mozilla/Attributes.h"
#include "mozilla/Maybe.h"
#include "nscore.h"
Expand Down Expand Up @@ -203,6 +204,15 @@ class nsTableWrapperFrame : public nsContainerFrame {
mozilla::LogicalPoint& aOrigin,
mozilla::WritingMode aWM) const;

// This is a helper for CreateReflowInputForInnerTable() and
// ComputeAutoSize(). It computes whether we need shrink-wrap behavior for
// children.
//
// Note: We don't need to call this in CreateReflowInputForCaption() because
// when we reflow the captions, we want them to stretch their inline-sizes to
// be at least as wide as the inner table frame.
mozilla::ComputeSizeFlags CreateComputeSizeFlagsForChild() const;

// Create and init the child reflow input, using passed-in aChildRI, so that
// caller can use it after we return.
//
Expand Down

0 comments on commit 05409a1

Please sign in to comment.