From e18c9b7ad6f2d59f6f0ffedbf6e5cfa53bae7d20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 1 Mar 2023 19:04:17 +0000 Subject: [PATCH] Bug 1815552 - Make positioned table parts deal correctly with switching position without being reframed. r=TYLin,layout-reviewers While looking at the backout, I noticed table parts relied on reframing on abspos-container-ness changes in a subtle way, see the test, which fails with the first patch of this bug applied without these changes. Make the NS_FRAME_CAN_HAVE_ABSPOS_CHILDREN mean the same for table parts as for everything else. Instead, keep the registration status on each relevant frame class individually. Depends on D169127 Differential Revision: https://phabricator.services.mozilla.com/D170969 --- layout/base/nsCSSFrameConstructor.cpp | 68 ++++++------------- layout/generic/nsIFrame.h | 1 - layout/generic/nsIFrameInlines.h | 6 +- layout/generic/nsSubDocumentFrame.cpp | 1 + layout/style/ComputedStyle.cpp | 24 ++++--- layout/style/ComputedStyle.h | 26 +++++++ layout/style/ComputedStyleInlines.h | 51 +++++++++++++- layout/style/nsStyleStruct.h | 27 -------- layout/style/nsStyleStructInlines.h | 54 --------------- layout/svg/SVGImageFrame.cpp | 1 + layout/tables/nsTableCellFrame.cpp | 6 +- layout/tables/nsTableFrame.cpp | 27 ++++++-- layout/tables/nsTableFrame.h | 18 +++-- layout/tables/nsTableRowFrame.cpp | 6 +- layout/tables/nsTableRowGroupFrame.cpp | 6 +- layout/xul/tree/nsTreeStyleCache.cpp | 1 + ...spos-container-change-dynamic-001-ref.html | 12 ++++ .../abspos-container-change-dynamic-001.html | 30 ++++++++ 18 files changed, 198 insertions(+), 167 deletions(-) create mode 100644 testing/web-platform/tests/css/css-tables/abspos-container-change-dynamic-001-ref.html create mode 100644 testing/web-platform/tests/css/css-tables/abspos-container-change-dynamic-001.html diff --git a/layout/base/nsCSSFrameConstructor.cpp b/layout/base/nsCSSFrameConstructor.cpp index 8a4798eec8aa..5ea94778eabf 100644 --- a/layout/base/nsCSSFrameConstructor.cpp +++ b/layout/base/nsCSSFrameConstructor.cpp @@ -2080,11 +2080,10 @@ nsIFrame* nsCSSFrameConstructor::ConstructTable(nsFrameConstructorState& aState, // Process children nsFrameConstructorSaveState absoluteSaveState; - const nsStyleDisplay* display = outerComputedStyle->StyleDisplay(); // Mark the table frame as an absolute container if needed newFrame->AddStateBits(NS_FRAME_CAN_HAVE_ABSPOS_CHILDREN); - if (display->IsAbsPosContainingBlock(newFrame)) { + if (newFrame->IsAbsPosContainingBlock()) { aState.PushAbsoluteContainingBlock(newFrame, newFrame, absoluteSaveState); } @@ -2117,21 +2116,14 @@ nsIFrame* nsCSSFrameConstructor::ConstructTable(nsFrameConstructorState& aState, return newFrame; } -static void MakeTablePartAbsoluteContainingBlockIfNeeded( - nsFrameConstructorState& aState, const nsStyleDisplay* aDisplay, - nsFrameConstructorSaveState& aAbsSaveState, nsContainerFrame* aFrame) { +static void MakeTablePartAbsoluteContainingBlock( + nsFrameConstructorState& aState, nsFrameConstructorSaveState& aAbsSaveState, + nsContainerFrame* aFrame) { // If we're positioned, then we need to become an absolute containing block - // for any absolutely positioned children and register for post-reflow fixup. - // - // Note that usually if a frame type can be an absolute containing block, we - // always set NS_FRAME_CAN_HAVE_ABSPOS_CHILDREN, whether it actually is or - // not. However, in this case flag serves the additional purpose of indicating - // that the frame was registered with its table frame. This allows us to avoid - // the overhead of unregistering the frame in most cases. - if (aDisplay->IsAbsPosContainingBlock(aFrame)) { - aFrame->AddStateBits(NS_FRAME_CAN_HAVE_ABSPOS_CHILDREN); + // for any absolutely positioned children. + aFrame->AddStateBits(NS_FRAME_CAN_HAVE_ABSPOS_CHILDREN); + if (aFrame->IsAbsPosContainingBlock()) { aState.PushAbsoluteContainingBlock(aFrame, aFrame, aAbsSaveState); - nsTableFrame::RegisterPositionedTablePart(aFrame); } } @@ -2162,8 +2154,7 @@ nsIFrame* nsCSSFrameConstructor::ConstructTableRowOrRowGroup( InitAndRestoreFrame(aState, content, aParentFrame, newFrame); nsFrameConstructorSaveState absoluteSaveState; - MakeTablePartAbsoluteContainingBlockIfNeeded(aState, aDisplay, - absoluteSaveState, newFrame); + MakeTablePartAbsoluteContainingBlock(aState, absoluteSaveState, newFrame); nsFrameConstructorSaveState floatSaveState; aState.MaybePushFloatContainingBlock(newFrame, floatSaveState); @@ -2262,8 +2253,7 @@ nsIFrame* nsCSSFrameConstructor::ConstructTableCell( InitAndRestoreFrame(aState, content, newFrame, cellInnerFrame); nsFrameConstructorSaveState absoluteSaveState; - MakeTablePartAbsoluteContainingBlockIfNeeded(aState, aDisplay, - absoluteSaveState, newFrame); + MakeTablePartAbsoluteContainingBlock(aState, absoluteSaveState, newFrame); nsFrameConstructorSaveState floatSaveState; aState.MaybePushFloatContainingBlock(cellInnerFrame, floatSaveState); @@ -2519,7 +2509,7 @@ nsIFrame* nsCSSFrameConstructor::ConstructDocElementFrame( processChildren = true; contentFrame->AddStateBits(NS_FRAME_CAN_HAVE_ABSPOS_CHILDREN); - if (display->IsAbsPosContainingBlock(contentFrame)) { + if (contentFrame->IsAbsPosContainingBlock()) { state.PushAbsoluteContainingBlock(contentFrame, contentFrame, absoluteSaveState); } @@ -2556,8 +2546,7 @@ nsIFrame* nsCSSFrameConstructor::ConstructDocElementFrame( state, aDocElement, state.GetGeometricParent(*display, mDocElementContainingBlock), mDocElementContainingBlock, computedStyle, &contentFrame, frameList, - display->IsAbsPosContainingBlock(contentFrame) ? contentFrame - : nullptr); + contentFrame->IsAbsPosContainingBlock() ? contentFrame : nullptr); } MOZ_ASSERT(frameList.FirstChild()); @@ -3306,7 +3295,7 @@ nsIFrame* nsCSSFrameConstructor::ConstructBlockRubyFrame( nsFrameConstructorSaveState absoluteSaveState; blockFrame->AddStateBits(NS_FRAME_CAN_HAVE_ABSPOS_CHILDREN); - if (aStyleDisplay->IsAbsPosContainingBlock(newFrame)) { + if (newFrame->IsAbsPosContainingBlock()) { aState.PushAbsoluteContainingBlock(blockFrame, blockFrame, absoluteSaveState); } @@ -3839,8 +3828,7 @@ void nsCSSFrameConstructor::ConstructFrameFromItemInternal( // Now figure out whether newFrame or outerFrame should be the // absolute container. - auto outerDisplay = outerStyle->StyleDisplay(); - if (outerDisplay->IsAbsPosContainingBlock(outerFrame)) { + if (outerFrame->IsAbsPosContainingBlock()) { maybeAbsoluteContainingBlock = outerFrame; maybeAbsoluteContainingBlockStyleFrame = outerFrame; innerFrame->AddStateBits(NS_FRAME_CAN_HAVE_ABSPOS_CHILDREN); @@ -4582,10 +4570,9 @@ nsIFrame* nsCSSFrameConstructor::ConstructScrollableBlock( aState.AddChild(newFrame, aFrameList, content, aParentFrame); nsFrameList blockList; - ConstructBlock( - aState, content, newFrame, newFrame, scrolledContentStyle, &scrolledFrame, - blockList, - aDisplay->IsAbsPosContainingBlock(newFrame) ? newFrame : nullptr); + ConstructBlock(aState, content, newFrame, newFrame, scrolledContentStyle, + &scrolledFrame, blockList, + newFrame->IsAbsPosContainingBlock() ? newFrame : nullptr); MOZ_ASSERT(blockList.OnlyChild() == scrolledFrame, "Scrollframe's frameList should be exactly the scrolled frame!"); @@ -4619,11 +4606,10 @@ nsIFrame* nsCSSFrameConstructor::ConstructNonScrollableBlock( nsContainerFrame* newFrame = NS_NewBlockFrame(mPresShell, computedStyle); newFrame->AddStateBits(flags); - ConstructBlock( - aState, aItem.mContent, - aState.GetGeometricParent(*aDisplay, aParentFrame), aParentFrame, - computedStyle, &newFrame, aFrameList, - aDisplay->IsAbsPosContainingBlock(newFrame) ? newFrame : nullptr); + ConstructBlock(aState, aItem.mContent, + aState.GetGeometricParent(*aDisplay, aParentFrame), + aParentFrame, computedStyle, &newFrame, aFrameList, + newFrame->IsAbsPosContainingBlock() ? newFrame : nullptr); return newFrame; } @@ -7894,9 +7880,8 @@ nsIFrame* nsCSSFrameConstructor::CreateContinuingTableFrame( headerFooterFrame->Init(headerFooter, newFrame, nullptr); nsFrameConstructorSaveState absoluteSaveState; - MakeTablePartAbsoluteContainingBlockIfNeeded( - state, headerFooterComputedStyle->StyleDisplay(), absoluteSaveState, - headerFooterFrame); + MakeTablePartAbsoluteContainingBlock(state, absoluteSaveState, + headerFooterFrame); nsFrameConstructorSaveState floatSaveState; state.MaybePushFloatContainingBlock(headerFooterFrame, floatSaveState); @@ -7969,16 +7954,10 @@ nsIFrame* nsCSSFrameConstructor::CreateContinuingFrame( } else if (LayoutFrameType::TableRowGroup == frameType) { newFrame = NS_NewTableRowGroupFrame(mPresShell, computedStyle); newFrame->Init(content, aParentFrame, aFrame); - if (newFrame->HasAnyStateBits(NS_FRAME_CAN_HAVE_ABSPOS_CHILDREN)) { - nsTableFrame::RegisterPositionedTablePart(newFrame); - } } else if (LayoutFrameType::TableRow == frameType) { nsTableRowFrame* rowFrame = NS_NewTableRowFrame(mPresShell, computedStyle); rowFrame->Init(content, aParentFrame, aFrame); - if (rowFrame->HasAnyStateBits(NS_FRAME_CAN_HAVE_ABSPOS_CHILDREN)) { - nsTableFrame::RegisterPositionedTablePart(rowFrame); - } // Create a continuing frame for each table cell frame nsFrameList newChildList; @@ -8007,9 +7986,6 @@ nsIFrame* nsCSSFrameConstructor::CreateContinuingFrame( NS_NewTableCellFrame(mPresShell, computedStyle, tableFrame); cellFrame->Init(content, aParentFrame, aFrame); - if (cellFrame->HasAnyStateBits(NS_FRAME_CAN_HAVE_ABSPOS_CHILDREN)) { - nsTableFrame::RegisterPositionedTablePart(cellFrame); - } // Create a continuing area frame nsIFrame* blockFrame = aFrame->PrincipalChildList().FirstChild(); diff --git a/layout/generic/nsIFrame.h b/layout/generic/nsIFrame.h index 4882723f08c9..c2e2fc5beccf 100644 --- a/layout/generic/nsIFrame.h +++ b/layout/generic/nsIFrame.h @@ -73,7 +73,6 @@ #include "nsStyleStruct.h" #include "Visibility.h" #include "nsChangeHint.h" -#include "mozilla/ComputedStyleInlines.h" #include "mozilla/EnumSet.h" #include "mozilla/gfx/2D.h" #include "mozilla/gfx/CompositorHitTestInfo.h" diff --git a/layout/generic/nsIFrameInlines.h b/layout/generic/nsIFrameInlines.h index 8296eaf8f3e4..e187f0dfb0bc 100644 --- a/layout/generic/nsIFrameInlines.h +++ b/layout/generic/nsIFrameInlines.h @@ -8,10 +8,10 @@ #define nsIFrameInlines_h___ #include "mozilla/dom/ElementInlines.h" +#include "mozilla/ComputedStyleInlines.h" #include "nsContainerFrame.h" #include "nsLayoutUtils.h" #include "nsPlaceholderFrame.h" -#include "nsStyleStructInlines.h" #include "nsCSSAnonBoxes.h" #include "nsFrameManager.h" @@ -53,11 +53,11 @@ bool nsIFrame::IsFloating() const { } bool nsIFrame::IsAbsPosContainingBlock() const { - return StyleDisplay()->IsAbsPosContainingBlock(this); + return Style()->IsAbsPosContainingBlock(this); } bool nsIFrame::IsFixedPosContainingBlock() const { - return StyleDisplay()->IsFixedPosContainingBlock(this); + return Style()->IsFixedPosContainingBlock(this); } bool nsIFrame::IsRelativelyOrStickyPositioned() const { diff --git a/layout/generic/nsSubDocumentFrame.cpp b/layout/generic/nsSubDocumentFrame.cpp index 06ccfedc17db..c236184fdb48 100644 --- a/layout/generic/nsSubDocumentFrame.cpp +++ b/layout/generic/nsSubDocumentFrame.cpp @@ -11,6 +11,7 @@ #include "nsSubDocumentFrame.h" +#include "mozilla/ComputedStyleInlines.h" #include "mozilla/Preferences.h" #include "mozilla/PresShell.h" #include "mozilla/StaticPrefs_layout.h" diff --git a/layout/style/ComputedStyle.cpp b/layout/style/ComputedStyle.cpp index 2d2cb33b7d31..ce96b2adcc6c 100644 --- a/layout/style/ComputedStyle.cpp +++ b/layout/style/ComputedStyle.cpp @@ -52,17 +52,18 @@ ComputedStyle::ComputedStyle(PseudoStyleType aPseudoType, // whether we establish a containing block has really changed. static bool ContainingBlockMayHaveChanged(const ComputedStyle& aOldStyle, const ComputedStyle& aNewStyle) { - auto* oldDisp = aOldStyle.StyleDisplay(); - auto* newDisp = aNewStyle.StyleDisplay(); + const auto& oldDisp = *aOldStyle.StyleDisplay(); + const auto& newDisp = *aNewStyle.StyleDisplay(); - if (oldDisp->IsPositionedStyle() != newDisp->IsPositionedStyle()) { + if (oldDisp.IsPositionedStyle() != newDisp.IsPositionedStyle()) { + // XXX This check can probably be moved to after the fixedCB check, since + // IsPositionedStyle() is also only relevant for non-svg text frame + // subtrees. return true; } - bool fixedCB = - oldDisp->IsFixedPosContainingBlockForNonSVGTextFrames(aOldStyle); - if (fixedCB != - newDisp->IsFixedPosContainingBlockForNonSVGTextFrames(aNewStyle)) { + const bool fixedCB = aOldStyle.IsFixedPosContainingBlockForNonSVGTextFrames(); + if (fixedCB != aNewStyle.IsFixedPosContainingBlockForNonSVGTextFrames()) { return true; } // If we were both before and after a fixed-pos containing-block that means @@ -71,20 +72,21 @@ static bool ContainingBlockMayHaveChanged(const ComputedStyle& aOldStyle, if (fixedCB) { return false; } + // Note that neither of these two following sets of frames // (transform-supporting and layout-and-paint-supporting frames) is a subset // of the other, because table frames support contain: layout/paint but not // transforms (which are instead inherited to the table wrapper), and quite a // few frame types support transforms but not contain: layout/paint (e.g., // table rows and row groups, many SVG frames). - if (oldDisp->IsFixedPosContainingBlockForTransformSupportingFrames() != - newDisp->IsFixedPosContainingBlockForTransformSupportingFrames()) { + if (oldDisp.IsFixedPosContainingBlockForTransformSupportingFrames() != + newDisp.IsFixedPosContainingBlockForTransformSupportingFrames()) { return true; } if (oldDisp - ->IsFixedPosContainingBlockForContainLayoutAndPaintSupportingFrames() != + .IsFixedPosContainingBlockForContainLayoutAndPaintSupportingFrames() != newDisp - ->IsFixedPosContainingBlockForContainLayoutAndPaintSupportingFrames()) { + .IsFixedPosContainingBlockForContainLayoutAndPaintSupportingFrames()) { return true; } return false; diff --git a/layout/style/ComputedStyle.h b/layout/style/ComputedStyle.h index e20b819f9337..95e0d363a4e2 100644 --- a/layout/style/ComputedStyle.h +++ b/layout/style/ComputedStyle.h @@ -229,6 +229,32 @@ class ComputedStyle { inline mozilla::StylePointerEvents PointerEvents() const; inline mozilla::StyleUserSelect UserSelect() const; + /** + * Returns whether the element is a containing block for its absolutely + * positioned descendants. + * aContextFrame is the frame for which this is the style (or an old style). + */ + inline bool IsAbsPosContainingBlock(const nsIFrame*) const; + + /** + * Returns true when the element is a containing block for its fixed-pos + * descendants. + * aContextFrame is the frame for which this is the style (or an old style). + */ + inline bool IsFixedPosContainingBlock(const nsIFrame*) const; + + /** + * Tests for only the sub-parts of IsFixedPosContainingBlock that apply to: + * - nearly all frames, except those that are in SVG text subtrees. + * - frames that support CSS contain:layout and contain:paint and are not + * in SVG text subtrees. + * - frames that support CSS transforms and are not in SVG text subtrees. + * + * This should be used only when the caller has the style but not the + * frame (i.e., when calculating style changes). + */ + inline bool IsFixedPosContainingBlockForNonSVGTextFrames() const; + /** * Compute the style changes needed during restyling when this style * context is being replaced by aNewContext. (This is nonsymmetric since diff --git a/layout/style/ComputedStyleInlines.h b/layout/style/ComputedStyleInlines.h index ad38d0b869bd..e208ce033c55 100644 --- a/layout/style/ComputedStyleInlines.h +++ b/layout/style/ComputedStyleInlines.h @@ -19,7 +19,7 @@ #include "MainThreadUtils.h" #include "mozilla/Assertions.h" #include "mozilla/Unused.h" -#include "nsStyleStruct.h" +#include "nsStyleStructInlines.h" namespace mozilla { @@ -68,6 +68,55 @@ StyleUserSelect ComputedStyle::UserSelect() const { : StyleUIReset()->ComputedUserSelect(); } +bool ComputedStyle::IsFixedPosContainingBlockForNonSVGTextFrames() const { + // NOTE: Any CSS properties that influence the output of this function + // should return FIXPOS_CB_NON_SVG for will-change. + if (IsRootElementStyle()) { + return false; + } + + const auto& disp = *StyleDisplay(); + if (disp.mWillChange.bits & mozilla::StyleWillChangeBits::FIXPOS_CB_NON_SVG) { + return true; + } + + const auto& effects = *StyleEffects(); + return effects.HasFilters() || effects.HasBackdropFilters(); +} + +bool ComputedStyle::IsFixedPosContainingBlock( + const nsIFrame* aContextFrame) const { + // NOTE: Any CSS properties that influence the output of this function + // should also handle will-change appropriately. + if (mozilla::SVGUtils::IsInSVGTextSubtree(aContextFrame)) { + return false; + } + if (IsFixedPosContainingBlockForNonSVGTextFrames()) { + return true; + } + const auto& disp = *StyleDisplay(); + if (disp.IsFixedPosContainingBlockForContainLayoutAndPaintSupportingFrames() && + aContextFrame->IsFrameOfType(nsIFrame::eSupportsContainLayoutAndPaint)) { + return true; + } + if (disp.IsFixedPosContainingBlockForTransformSupportingFrames() && + aContextFrame->IsFrameOfType(nsIFrame::eSupportsCSSTransforms)) { + return true; + } + return false; +} + +bool ComputedStyle::IsAbsPosContainingBlock( + const nsIFrame* aContextFrame) const { + if (IsFixedPosContainingBlock(aContextFrame)) { + return true; + } + // NOTE: Any CSS properties that influence the output of this function + // should also handle will-change appropriately. + return StyleDisplay()->IsPositionedStyle() && + !mozilla::SVGUtils::IsInSVGTextSubtree(aContextFrame); +} + } // namespace mozilla #endif // ComputedStyleInlines_h diff --git a/layout/style/nsStyleStruct.h b/layout/style/nsStyleStruct.h index 410d341ac796..1caabcf4858e 100644 --- a/layout/style/nsStyleStruct.h +++ b/layout/style/nsStyleStruct.h @@ -1730,33 +1730,6 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleDisplay { */ inline bool HasPerspective(const nsIFrame* aContextFrame) const; - /** - * Returns whether the element is a containing block for its - * absolutely positioned descendants. - * aContextFrame is the frame for which this is the nsStyleDisplay. - */ - inline bool IsAbsPosContainingBlock(const nsIFrame* aContextFrame) const; - - /** - * Returns true when the element is a containing block for its fixed-pos - * descendants. - * aContextFrame is the frame for which this is the nsStyleDisplay. - */ - inline bool IsFixedPosContainingBlock(const nsIFrame* aContextFrame) const; - - /** - * Tests for only the sub-parts of IsFixedPosContainingBlock that apply - * to: - * - nearly all frames, except those that are SVG text frames. - * - frames that support CSS contain:layout and contain:paint and are not - * SVG text frames. - * - frames that support CSS transforms and are not SVG text frames. - * - * This should be used only when the caller has the style but not the - * frame (i.e., when calculating style changes). - */ - inline bool IsFixedPosContainingBlockForNonSVGTextFrames( - const mozilla::ComputedStyle&) const; inline bool IsFixedPosContainingBlockForContainLayoutAndPaintSupportingFrames() const; inline bool IsFixedPosContainingBlockForTransformSupportingFrames() const; diff --git a/layout/style/nsStyleStructInlines.h b/layout/style/nsStyleStructInlines.h index c5e2eaee2aa6..a15298a83de6 100644 --- a/layout/style/nsStyleStructInlines.h +++ b/layout/style/nsStyleStructInlines.h @@ -91,24 +91,6 @@ bool nsStyleDisplay::HasPerspective(const nsIFrame* aContextFrame) const { aContextFrame->IsFrameOfType(nsIFrame::eSupportsCSSTransforms); } -bool nsStyleDisplay::IsFixedPosContainingBlockForNonSVGTextFrames( - const mozilla::ComputedStyle& aStyle) const { - // NOTE: Any CSS properties that influence the output of this function - // should return FIXPOS_CB_NON_SVG for will-change. - NS_ASSERTION(aStyle.StyleDisplay() == this, "unexpected aStyle"); - - if (aStyle.IsRootElementStyle()) { - return false; - } - - if (mWillChange.bits & mozilla::StyleWillChangeBits::FIXPOS_CB_NON_SVG) { - return true; - } - - return aStyle.StyleEffects()->HasFilters() || - aStyle.StyleEffects()->HasBackdropFilters(); -} - bool nsStyleDisplay:: IsFixedPosContainingBlockForContainLayoutAndPaintSupportingFrames() const { return IsContainPaint() || IsContainLayout() || @@ -123,42 +105,6 @@ bool nsStyleDisplay::IsFixedPosContainingBlockForTransformSupportingFrames() mWillChange.bits & mozilla::StyleWillChangeBits::PERSPECTIVE; } -bool nsStyleDisplay::IsFixedPosContainingBlock( - const nsIFrame* aContextFrame) const { - const auto* style = aContextFrame->Style(); - NS_ASSERTION(style->StyleDisplay() == this, "unexpected aContextFrame"); - // NOTE: Any CSS properties that influence the output of this function - // should also handle will-change appropriately. - if (mozilla::SVGUtils::IsInSVGTextSubtree(aContextFrame)) { - return false; - } - if (IsFixedPosContainingBlockForNonSVGTextFrames(*style)) { - return true; - } - if (IsFixedPosContainingBlockForContainLayoutAndPaintSupportingFrames() && - aContextFrame->IsFrameOfType(nsIFrame::eSupportsContainLayoutAndPaint)) { - return true; - } - if (IsFixedPosContainingBlockForTransformSupportingFrames() && - aContextFrame->IsFrameOfType(nsIFrame::eSupportsCSSTransforms)) { - return true; - } - return false; -} - -bool nsStyleDisplay::IsAbsPosContainingBlock( - const nsIFrame* aContextFrame) const { - NS_ASSERTION(aContextFrame->StyleDisplay() == this, - "unexpected aContextFrame"); - if (IsFixedPosContainingBlock(aContextFrame)) { - return true; - } - // NOTE: Any CSS properties that influence the output of this function - // should also handle will-change appropriately. - return IsPositionedStyle() && - !mozilla::SVGUtils::IsInSVGTextSubtree(aContextFrame); -} - bool nsStyleDisplay::IsRelativelyOrStickyPositioned( const nsIFrame* aContextFrame) const { NS_ASSERTION(aContextFrame->StyleDisplay() == this, diff --git a/layout/svg/SVGImageFrame.cpp b/layout/svg/SVGImageFrame.cpp index 2006ea7f998f..2ad43c6b766e 100644 --- a/layout/svg/SVGImageFrame.cpp +++ b/layout/svg/SVGImageFrame.cpp @@ -10,6 +10,7 @@ #include "gfxContext.h" #include "gfxPlatform.h" #include "mozilla/gfx/2D.h" +#include "mozilla/ComputedStyleInlines.h" #include "mozilla/image/WebRenderImageProvider.h" #include "mozilla/layers/RenderRootStateManager.h" #include "mozilla/layers/WebRenderLayerManager.h" diff --git a/layout/tables/nsTableCellFrame.cpp b/layout/tables/nsTableCellFrame.cpp index a9e1f1f73086..133e2c9bbdf4 100644 --- a/layout/tables/nsTableCellFrame.cpp +++ b/layout/tables/nsTableCellFrame.cpp @@ -83,10 +83,7 @@ void nsTableCellFrame::Init(nsIContent* aContent, nsContainerFrame* aParent, void nsTableCellFrame::DestroyFrom(nsIFrame* aDestructRoot, PostDestroyData& aPostDestroyData) { - if (HasAnyStateBits(NS_FRAME_CAN_HAVE_ABSPOS_CHILDREN)) { - nsTableFrame::UnregisterPositionedTablePart(this, aDestructRoot); - } - + nsTableFrame::MaybeUnregisterPositionedTablePart(this, aDestructRoot); nsContainerFrame::DestroyFrom(aDestructRoot, aPostDestroyData); } @@ -183,6 +180,7 @@ nsresult nsTableCellFrame::AttributeChanged(int32_t aNameSpaceID, /* virtual */ void nsTableCellFrame::DidSetComputedStyle(ComputedStyle* aOldComputedStyle) { nsContainerFrame::DidSetComputedStyle(aOldComputedStyle); + nsTableFrame::PositionedTablePartMaybeChanged(this, aOldComputedStyle); if (!aOldComputedStyle) { return; // avoid the following on init diff --git a/layout/tables/nsTableFrame.cpp b/layout/tables/nsTableFrame.cpp index 60f01140a178..686502624798 100644 --- a/layout/tables/nsTableFrame.cpp +++ b/layout/tables/nsTableFrame.cpp @@ -18,6 +18,7 @@ #include "gfxContext.h" #include "nsCOMPtr.h" #include "mozilla/ComputedStyle.h" +#include "nsIFrameInlines.h" #include "nsFrameList.h" #include "nsStyleConsts.h" #include "nsIContent.h" @@ -250,7 +251,16 @@ bool nsTableFrame::PageBreakAfter(nsIFrame* aSourceFrame, } /* static */ -void nsTableFrame::RegisterPositionedTablePart(nsIFrame* aFrame) { +void nsTableFrame::PositionedTablePartMaybeChanged(nsIFrame* aFrame, + ComputedStyle* aOldStyle) { + const bool wasPositioned = + aOldStyle && aOldStyle->IsAbsPosContainingBlock(aFrame); + const bool isPositioned = aFrame->IsAbsPosContainingBlock(); + MOZ_ASSERT(isPositioned == aFrame->Style()->IsAbsPosContainingBlock(aFrame)); + if (wasPositioned == isPositioned) { + return; + } + nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(aFrame); MOZ_ASSERT(tableFrame, "Should have a table frame here"); tableFrame = static_cast(tableFrame->FirstContinuation()); @@ -265,13 +275,20 @@ void nsTableFrame::RegisterPositionedTablePart(nsIFrame* aFrame) { tableFrame->SetProperty(PositionedTablePartArray(), positionedParts); } - // Add this frame to the list. - positionedParts->AppendElement(aFrame); + if (isPositioned) { + // Add this frame to the list. + positionedParts->AppendElement(aFrame); + } else { + positionedParts->RemoveElement(aFrame); + } } /* static */ -void nsTableFrame::UnregisterPositionedTablePart(nsIFrame* aFrame, - nsIFrame* aDestructRoot) { +void nsTableFrame::MaybeUnregisterPositionedTablePart(nsIFrame* aFrame, + nsIFrame* aDestructRoot) { + if (!aFrame->IsAbsPosContainingBlock()) { + return; + } // Retrieve the table frame, and check if we hit aDestructRoot on the way. bool didPassThrough; nsTableFrame* tableFrame = diff --git a/layout/tables/nsTableFrame.h b/layout/tables/nsTableFrame.h index 00336c3fb941..fe28abce6f4f 100644 --- a/layout/tables/nsTableFrame.h +++ b/layout/tables/nsTableFrame.h @@ -177,14 +177,18 @@ class nsTableFrame : public nsContainerFrame { static bool PageBreakAfter(nsIFrame* aSourceFrame, nsIFrame* aNextFrame); - // Register a positioned table part with its nsTableFrame. These objects will - // be visited by FixupPositionedTableParts after reflow is complete. (See that - // function for more explanation.) Should be called during frame construction. - static void RegisterPositionedTablePart(nsIFrame* aFrame); + // Register or deregister a positioned table part with its nsTableFrame. + // These objects will be visited by FixupPositionedTableParts after reflow is + // complete. (See that function for more explanation.) Should be called + // during frame construction or style recalculation. + // + // @return true if the frame is a registered positioned table part. + static void PositionedTablePartMaybeChanged( + nsIFrame*, mozilla::ComputedStyle* aOldStyle); - // Unregister a positioned table part with its nsTableFrame. - static void UnregisterPositionedTablePart(nsIFrame* aFrame, - nsIFrame* aDestructRoot); + // Unregister a positioned table part with its nsTableFrame, if needed. + static void MaybeUnregisterPositionedTablePart(nsIFrame* aFrame, + nsIFrame* aDestructRoot); /* * Notification that rowspan or colspan has changed for content inside a diff --git a/layout/tables/nsTableRowFrame.cpp b/layout/tables/nsTableRowFrame.cpp index 30f4b4dcc9cd..89302ca9a6e8 100644 --- a/layout/tables/nsTableRowFrame.cpp +++ b/layout/tables/nsTableRowFrame.cpp @@ -156,16 +156,14 @@ void nsTableRowFrame::Init(nsIContent* aContent, nsContainerFrame* aParent, void nsTableRowFrame::DestroyFrom(nsIFrame* aDestructRoot, PostDestroyData& aPostDestroyData) { - if (HasAnyStateBits(NS_FRAME_CAN_HAVE_ABSPOS_CHILDREN)) { - nsTableFrame::UnregisterPositionedTablePart(this, aDestructRoot); - } - + nsTableFrame::MaybeUnregisterPositionedTablePart(this, aDestructRoot); nsContainerFrame::DestroyFrom(aDestructRoot, aPostDestroyData); } /* virtual */ void nsTableRowFrame::DidSetComputedStyle(ComputedStyle* aOldComputedStyle) { nsContainerFrame::DidSetComputedStyle(aOldComputedStyle); + nsTableFrame::PositionedTablePartMaybeChanged(this, aOldComputedStyle); if (!aOldComputedStyle) { return; // avoid the following on init diff --git a/layout/tables/nsTableRowGroupFrame.cpp b/layout/tables/nsTableRowGroupFrame.cpp index fc8abb3e3a53..2c1c3ee5956b 100644 --- a/layout/tables/nsTableRowGroupFrame.cpp +++ b/layout/tables/nsTableRowGroupFrame.cpp @@ -64,10 +64,7 @@ nsTableRowGroupFrame::~nsTableRowGroupFrame() = default; void nsTableRowGroupFrame::DestroyFrom(nsIFrame* aDestructRoot, PostDestroyData& aPostDestroyData) { - if (HasAnyStateBits(NS_FRAME_CAN_HAVE_ABSPOS_CHILDREN)) { - nsTableFrame::UnregisterPositionedTablePart(this, aDestructRoot); - } - + nsTableFrame::MaybeUnregisterPositionedTablePart(this, aDestructRoot); nsContainerFrame::DestroyFrom(aDestructRoot, aPostDestroyData); } @@ -1436,6 +1433,7 @@ bool nsTableRowGroupFrame::ComputeCustomOverflow( void nsTableRowGroupFrame::DidSetComputedStyle( ComputedStyle* aOldComputedStyle) { nsContainerFrame::DidSetComputedStyle(aOldComputedStyle); + nsTableFrame::PositionedTablePartMaybeChanged(this, aOldComputedStyle); if (!aOldComputedStyle) { return; // avoid the following on init diff --git a/layout/xul/tree/nsTreeStyleCache.cpp b/layout/xul/tree/nsTreeStyleCache.cpp index 108d808aabd6..d5823ac38e98 100644 --- a/layout/xul/tree/nsTreeStyleCache.cpp +++ b/layout/xul/tree/nsTreeStyleCache.cpp @@ -5,6 +5,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ #include "nsTreeStyleCache.h" +#include "mozilla/ComputedStyleInlines.h" #include "mozilla/dom/Element.h" #include "mozilla/ServoStyleSet.h" #include "nsPresContextInlines.h" diff --git a/testing/web-platform/tests/css/css-tables/abspos-container-change-dynamic-001-ref.html b/testing/web-platform/tests/css/css-tables/abspos-container-change-dynamic-001-ref.html new file mode 100644 index 000000000000..606021ecbe0d --- /dev/null +++ b/testing/web-platform/tests/css/css-tables/abspos-container-change-dynamic-001-ref.html @@ -0,0 +1,12 @@ + + +CSS Test Reference + + + + + + +
AB +
+
diff --git a/testing/web-platform/tests/css/css-tables/abspos-container-change-dynamic-001.html b/testing/web-platform/tests/css/css-tables/abspos-container-change-dynamic-001.html new file mode 100644 index 000000000000..5fdf7ce96759 --- /dev/null +++ b/testing/web-platform/tests/css/css-tables/abspos-container-change-dynamic-001.html @@ -0,0 +1,30 @@ + + +Dynamic position change in table cell with abspos child + + + + + + + + + + + +
AB
+