tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit 5e022982db6bb7dac68a0c8ec056fedce56869a6
parent f9b0bf22162805b9bd6268265a0129a1948dd631
Author: Emilio Cobos Álvarez <emilio@crisal.io>
Date:   Sat,  3 Jan 2026 09:26:46 +0000

Bug 2008148 - Unify frame constructor append and insertion codepaths. r=TYLin,layout-reviewers

Having all that duplicated code is kinda useless, since append needs to
deal with insert anyways due to things like `::after` and
`display: contents`. This fixes the issue by virtue of this check:

  https://searchfox.org/firefox-main/rev/aee7c0f24f488cd7f5a835803b48dd0c0cb2fd5f/layout/base/nsCSSFrameConstructor.cpp#7009

Which is really what's going on. IsValidSibling is wrong and can go.
Instead let WipeContainingBlock* deal with it, since we run that after
constructing the items.

This unlocks further simplifications but seems like a step in the right
direction and I don't want to get too side tracked into rewriting the
frame constructor.

The fieldset frame change mirrors table captions. Legends are dealt with
properly by the existing check in ConstructFramesFromItemList and / or
WipeContainingBlock. That allows to remove some special cases in
nsFlexContainerFrame and InspectorUtils.

Differential Revision: https://phabricator.services.mozilla.com/D277777

Diffstat:
Mlayout/base/crashtests/crashtests.list | 2+-
Mlayout/base/nsCSSFrameConstructor.cpp | 731++++++++++++++-----------------------------------------------------------------
Mlayout/base/nsCSSFrameConstructor.h | 94+++++++++++++++++++------------------------------------------------------------
Mlayout/forms/nsFieldSetFrame.h | 7+++++++
Mlayout/generic/nsFlexContainerFrame.cpp | 25+++++--------------------
Mlayout/inspector/InspectorUtils.cpp | 7-------
Atesting/web-platform/tests/css/css-tables/table-caption-after-crash.html | 10++++++++++
7 files changed, 169 insertions(+), 707 deletions(-)

diff --git a/layout/base/crashtests/crashtests.list b/layout/base/crashtests/crashtests.list @@ -200,7 +200,7 @@ load 411870-1.html load 412651-1.html load 413587-1.svg load 415503.xhtml -asserts(1-1) load 416107.xhtml # bug 489100, ASSERTION: Out-of-flow frame got reflowed before its placeholder +load 416107.xhtml HTTP load 419985.html load 420031-1.html load 420213-1.html diff --git a/layout/base/nsCSSFrameConstructor.cpp b/layout/base/nsCSSFrameConstructor.cpp @@ -5666,82 +5666,9 @@ void nsCSSFrameConstructor::AppendFramesToParent( std::move(aFrameList)); } -// This gets called to see if the frames corresponding to aSibling and aContent -// should be siblings in the frame tree. Although (1) rows and cols, (2) row -// groups and col groups, (3) row groups and captions, (4) legends and content -// inside fieldsets, (5) popups and other kids of the menu are siblings from a -// content perspective, they are not considered siblings in the frame tree. -bool nsCSSFrameConstructor::IsValidSibling(nsIFrame* aSibling, - nsIContent* aContent, - Maybe<StyleDisplay>& aDisplay) { - StyleDisplay siblingDisplay = aSibling->GetDisplay(); - if (StyleDisplay::TableColumnGroup == siblingDisplay || - StyleDisplay::TableColumn == siblingDisplay || - StyleDisplay::TableCaption == siblingDisplay || - StyleDisplay::TableHeaderGroup == siblingDisplay || - StyleDisplay::TableRowGroup == siblingDisplay || - StyleDisplay::TableFooterGroup == siblingDisplay) { - // if we haven't already, resolve a style to find the display type of - // aContent. - if (aDisplay.isNothing()) { - if (aContent->IsComment() || aContent->IsProcessingInstruction()) { - // Comments and processing instructions never have frames, so we should - // not try to generate styles for them. - return false; - } - // FIXME(emilio): This is buggy some times, see bug 1424656. - RefPtr<ComputedStyle> computedStyle = ResolveComputedStyle(aContent); - const nsStyleDisplay* display = computedStyle->StyleDisplay(); - aDisplay.emplace(display->mDisplay); - } - - StyleDisplay display = aDisplay.value(); - // To have decent performance we want to return false in cases in which - // reordering the two siblings has no effect on display. To ensure - // correctness, we MUST return false in cases where the two siblings have - // the same desired parent type and live on different display lists. - // Specificaly, columns and column groups should only consider columns and - // column groups as valid siblings. Captions should only consider other - // captions. All other things should consider each other as valid - // siblings. The restriction in the |if| above on siblingDisplay is ok, - // because for correctness the only part that really needs to happen is to - // not consider captions, column groups, and row/header/footer groups - // siblings of each other. Treating a column or colgroup as a valid - // sibling of a non-table-related frame will just mean we end up reframing. - if ((siblingDisplay == StyleDisplay::TableCaption) != - (display == StyleDisplay::TableCaption)) { - // One's a caption and the other is not. Not valid siblings. - return false; - } - - if ((siblingDisplay == StyleDisplay::TableColumnGroup || - siblingDisplay == StyleDisplay::TableColumn) != - (display == StyleDisplay::TableColumnGroup || - display == StyleDisplay::TableColumn)) { - // One's a column or column group and the other is not. Not valid - // siblings. - return false; - } - // Fall through; it's possible that the display type was overridden and - // a different sort of frame was constructed, so we may need to return false - // below. - } - - return true; -} - -// FIXME(emilio): If we ever kill IsValidSibling() we can simplify this quite a -// bit (no need to pass aTargetContent or aTargetContentDisplay, and the -// adjust() calls can be responsibility of the caller). template <nsCSSFrameConstructor::SiblingDirection aDirection> nsIFrame* nsCSSFrameConstructor::FindSiblingInternal( - FlattenedChildIterator& aIter, nsIContent* aTargetContent, - Maybe<StyleDisplay>& aTargetContentDisplay) { - auto adjust = [&](nsIFrame* aPotentialSiblingFrame) -> nsIFrame* { - return AdjustSiblingFrame(aPotentialSiblingFrame, aTargetContent, - aTargetContentDisplay, aDirection); - }; - + FlattenedChildIterator& aIter) { auto nextDomSibling = [](FlattenedChildIterator& aIter) -> nsIContent* { return aDirection == SiblingDirection::Forward ? aIter.GetNextChild() : aIter.GetPreviousChild(); @@ -5788,42 +5715,40 @@ nsIFrame* nsCSSFrameConstructor::FindSiblingInternal( if (nsIFrame* primaryFrame = sibling->GetPrimaryFrame()) { // XXX the GetContent() == sibling check is needed due to bug 135040. // Remove it once that's fixed. - if (primaryFrame->GetContent() == sibling) { - if (nsIFrame* frame = adjust(primaryFrame)) { - return frame; - } + // The rendered legend check is needed because it is conceptually out of + // flow. There can only be one rendered legend at a time, and we reframe + // when that changes, so this is ok. Table captions are a similar case, + // but there can be multiple of them and they instead get pulled out (and + // the sibling fixed up) after building the frame list. + if (primaryFrame->GetContent() == sibling && + !primaryFrame->IsRenderedLegend()) [[likely]] { + return primaryFrame; } } if (IsDisplayContents(sibling)) { - if (nsIFrame* frame = adjust(getNearPseudo(sibling))) { + if (nsIFrame* frame = getNearPseudo(sibling)) { return frame; } const bool startFromBeginning = aDirection == SiblingDirection::Forward; FlattenedChildIterator iter(sibling, startFromBeginning); - nsIFrame* sibling = FindSiblingInternal<aDirection>( - iter, aTargetContent, aTargetContentDisplay); + nsIFrame* sibling = FindSiblingInternal<aDirection>(iter); if (sibling) { return sibling; } } } - return adjust(getFarPseudo(aIter.Parent())); + return getFarPseudo(aIter.Parent()); } nsIFrame* nsCSSFrameConstructor::AdjustSiblingFrame( - nsIFrame* aSibling, nsIContent* aTargetContent, - Maybe<StyleDisplay>& aTargetContentDisplay, SiblingDirection aDirection) { + nsIFrame* aSibling, SiblingDirection aDirection) { if (!aSibling) { return nullptr; } - if (aSibling->IsRenderedLegend()) { - return nullptr; - } - if (aSibling->HasAnyStateBits(NS_FRAME_OUT_OF_FLOW)) { aSibling = aSibling->GetPlaceholderFrame(); MOZ_ASSERT(aSibling); @@ -5842,33 +5767,26 @@ nsIFrame* nsCSSFrameConstructor::AdjustSiblingFrame( aSibling = aSibling->GetTailContinuation(); } - if (!IsValidSibling(aSibling, aTargetContent, aTargetContentDisplay)) { - return nullptr; - } - return aSibling; } nsIFrame* nsCSSFrameConstructor::FindPreviousSibling( - const FlattenedChildIterator& aIter, - Maybe<StyleDisplay>& aTargetContentDisplay) { - return FindSibling<SiblingDirection::Backward>(aIter, aTargetContentDisplay); + const FlattenedChildIterator& aIter) { + return AdjustSiblingFrame(FindSibling<SiblingDirection::Backward>(aIter), + SiblingDirection::Backward); } nsIFrame* nsCSSFrameConstructor::FindNextSibling( - const FlattenedChildIterator& aIter, - Maybe<StyleDisplay>& aTargetContentDisplay) { - return FindSibling<SiblingDirection::Forward>(aIter, aTargetContentDisplay); + const FlattenedChildIterator& aIter) { + return AdjustSiblingFrame(FindSibling<SiblingDirection::Forward>(aIter), + SiblingDirection::Forward); } template <nsCSSFrameConstructor::SiblingDirection aDirection> nsIFrame* nsCSSFrameConstructor::FindSibling( - const FlattenedChildIterator& aIter, - Maybe<StyleDisplay>& aTargetContentDisplay) { - nsIContent* targetContent = aIter.Get(); + const FlattenedChildIterator& aIter) { FlattenedChildIterator siblingIter = aIter; - nsIFrame* sibling = FindSiblingInternal<aDirection>( - siblingIter, targetContent, aTargetContentDisplay); + nsIFrame* sibling = FindSiblingInternal<aDirection>(siblingIter); if (sibling) { return sibling; } @@ -5883,8 +5801,7 @@ nsIFrame* nsCSSFrameConstructor::FindSibling( FlattenedChildIterator iter(parent); iter.Seek(current); - sibling = FindSiblingInternal<aDirection>(iter, targetContent, - aTargetContentDisplay); + sibling = FindSiblingInternal<aDirection>(iter); if (sibling) { return sibling; } @@ -5895,29 +5812,8 @@ nsIFrame* nsCSSFrameConstructor::FindSibling( return nullptr; } -// For fieldsets, returns the area frame, if the child is not a legend. -static nsContainerFrame* GetAdjustedParentFrame(nsContainerFrame* aParentFrame, - nsIContent* aChildContent) { - MOZ_ASSERT(!aParentFrame->IsTableWrapperFrame(), "Shouldn't be happening!"); - - nsContainerFrame* newParent = nullptr; - if (aParentFrame->IsFieldSetFrame()) { - // If the parent is a fieldSet, use the fieldSet's area frame as the - // parent unless the new content is a legend. - if (!aChildContent->IsHTMLElement(nsGkAtoms::legend)) { - newParent = static_cast<nsFieldSetFrame*>(aParentFrame)->GetInner(); - if (newParent) { - newParent = newParent->GetContentInsertionFrame(); - } - } - } - return newParent ? newParent : aParentFrame; -} - nsIFrame* nsCSSFrameConstructor::GetInsertionPrevSibling( - InsertionPoint* aInsertion, nsIContent* aChild, bool* aIsAppend, - bool* aIsRangeInsertSafe, nsIContent* aStartSkipChild, - nsIContent* aEndSkipChild) { + InsertionPoint* aInsertion, nsIContent* aChild, bool* aIsAppend) { MOZ_ASSERT(aInsertion->mParentFrame, "Must have parent frame to start with"); *aIsAppend = false; @@ -5928,11 +5824,7 @@ nsIFrame* nsCSSFrameConstructor::GetInsertionPrevSibling( // The check for IsRootOfNativeAnonymousSubtree() is because editor is // severely broken and calls us directly for native anonymous // nodes that it creates. - if (aStartSkipChild) { - iter.Seek(aStartSkipChild); - } else { - iter.Seek(aChild); - } + iter.Seek(aChild); } else { // Prime the iterator for the call to FindPreviousSibling. iter.GetNextChild(); @@ -5943,8 +5835,7 @@ nsIFrame* nsCSSFrameConstructor::GetInsertionPrevSibling( // Note that FindPreviousSibling is passed the iterator by value, so that // the later usage of the iterator starts from the same place. - Maybe<StyleDisplay> childDisplay; - nsIFrame* prevSibling = FindPreviousSibling(iter, childDisplay); + nsIFrame* prevSibling = FindPreviousSibling(iter); // Now, find the geometric parent so that we can handle // continuations properly. Use the prev sibling if we have it; @@ -5952,33 +5843,25 @@ nsIFrame* nsCSSFrameConstructor::GetInsertionPrevSibling( if (prevSibling) { aInsertion->mParentFrame = prevSibling->GetParent()->GetContentInsertionFrame(); - } else { + *aIsAppend = + !::GetInsertNextSibling(aInsertion->mParentFrame, prevSibling) && + !nsLayoutUtils::GetNextContinuationOrIBSplitSibling( + aInsertion->mParentFrame) && + !IsWrapperPseudo(aInsertion->mParentFrame); + } else if (nsIFrame* nextSibling = FindNextSibling(iter)) { // If there is no previous sibling, then find the frame that follows - // - // FIXME(emilio): This is really complex and probably shouldn't be. - if (aEndSkipChild) { - iter.Seek(aEndSkipChild); - iter.GetPreviousChild(); - } - if (nsIFrame* nextSibling = FindNextSibling(iter, childDisplay)) { - aInsertion->mParentFrame = - nextSibling->GetParent()->GetContentInsertionFrame(); - } else { - // No previous or next sibling, so treat this like an appended frame. - *aIsAppend = true; - - // Deal with fieldsets. - aInsertion->mParentFrame = - ::GetAdjustedParentFrame(aInsertion->mParentFrame, aChild); + aInsertion->mParentFrame = + nextSibling->GetParent()->GetContentInsertionFrame(); + } else { + // No previous or next sibling, so treat this like an appended frame. + *aIsAppend = true; - aInsertion->mParentFrame = - ::ContinuationToAppendTo(aInsertion->mParentFrame); + aInsertion->mParentFrame = + ::ContinuationToAppendTo(aInsertion->mParentFrame); - prevSibling = ::FindAppendPrevSibling(aInsertion->mParentFrame, nullptr); - } + prevSibling = ::FindAppendPrevSibling(aInsertion->mParentFrame, nullptr); } - *aIsRangeInsertSafe = childDisplay.isNothing(); return prevSibling; } @@ -6097,16 +5980,16 @@ void nsCSSFrameConstructor::CheckBitsForLazyFrameConstruction( // Returns true if this operation can be lazy, false if not. // -// FIXME(emilio, bug 1410020): This function assumes that the flattened tree -// parent of all the appended children is the same, which, afaict, is not -// necessarily true. -void nsCSSFrameConstructor::ConstructLazily(Operation aOperation, - nsIContent* aChild) { - MOZ_ASSERT(aChild->GetParent()); +// NOTE(emilio): This function assumes that the flattened tree +// parent of all the appended children is the same, which holds because this +// gets called after GetRangeInsertionPoint which notifies individually. +void nsCSSFrameConstructor::ConstructLazily(nsIContent* aStartChild, + nsIContent* aEndChild) { + MOZ_ASSERT(aStartChild->GetParent()); // We can construct lazily; just need to set suitable bits in the content // tree. - Element* parent = aChild->GetFlattenedTreeParentElement(); + Element* parent = aStartChild->GetFlattenedTreeParentElement(); if (!parent) { // Not part of the flat tree, nothing to do. return; @@ -6122,24 +6005,15 @@ void nsCSSFrameConstructor::ConstructLazily(Operation aOperation, } // Set NODE_NEEDS_FRAME on the new nodes. - if (aOperation == CONTENTINSERT) { - NS_ASSERTION(!aChild->GetPrimaryFrame() || - aChild->GetPrimaryFrame()->GetContent() != aChild, - // XXX the aChild->GetPrimaryFrame()->GetContent() != aChild + for (nsIContent* child = aStartChild; child != aEndChild; + child = child->GetNextSibling()) { + NS_ASSERTION(!child->GetPrimaryFrame() || + child->GetPrimaryFrame()->GetContent() != child, + // XXX the child->GetPrimaryFrame()->GetContent() != child // check is needed due to bug 135040. Remove it once that's // fixed. "setting NEEDS_FRAME on a node that already has a frame?"); - aChild->SetFlags(NODE_NEEDS_FRAME); - } else { // CONTENTAPPEND - for (nsIContent* child = aChild; child; child = child->GetNextSibling()) { - NS_ASSERTION(!child->GetPrimaryFrame() || - child->GetPrimaryFrame()->GetContent() != child, - // XXX the child->GetPrimaryFrame()->GetContent() != child - // check is needed due to bug 135040. Remove it once that's - // fixed. - "setting NEEDS_FRAME on a node that already has a frame?"); - child->SetFlags(NODE_NEEDS_FRAME); - } + child->SetFlags(NODE_NEEDS_FRAME); } CheckBitsForLazyFrameConstruction(parent); @@ -6161,12 +6035,6 @@ void nsCSSFrameConstructor::IssueSingleInsertNofications( } } -bool nsCSSFrameConstructor::InsertionPoint::IsMultiple() const { - // Fieldset frames have multiple normal flow child frame lists so handle it - // the same as if it had multiple content insertion points. - return mParentFrame && mParentFrame->IsFieldSetFrame(); -} - nsCSSFrameConstructor::InsertionPoint nsCSSFrameConstructor::GetRangeInsertionPoint(nsIContent* aStartChild, nsIContent* aEndChild, @@ -6198,15 +6066,8 @@ nsCSSFrameConstructor::GetRangeInsertionPoint(nsIContent* aStartChild, #endif // Now the flattened tree parent of all the siblings is the same, just use the - // same insertion point and take the fast path, unless it's a multiple - // insertion point. - InsertionPoint ip = GetInsertionPoint(aStartChild); - if (ip.IsMultiple()) { - IssueSingleInsertNofications(aStartChild, aEndChild, aInsertionKind); - return {}; - } - - return ip; + // same insertion point. + return GetInsertionPoint(aStartChild); } bool nsCSSFrameConstructor::MaybeRecreateForFrameset(nsIFrame* aParentFrame, @@ -6284,25 +6145,6 @@ void nsCSSFrameConstructor::StyleNewChildRange(nsIContent* aStartChild, } } -nsIFrame* nsCSSFrameConstructor::FindNextSiblingForAppend( - const InsertionPoint& aInsertion) { - auto SlowPath = [&]() -> nsIFrame* { - FlattenedChildIterator iter(aInsertion.mContainer, - /* aStartAtBeginning = */ false); - iter.GetPreviousChild(); // Prime the iterator. - Maybe<StyleDisplay> unused; - return FindNextSibling(iter, unused); - }; - - if (!IsDisplayContents(aInsertion.mContainer) && - !nsLayoutUtils::GetAfterFrame(aInsertion.mContainer)) { - MOZ_ASSERT(!SlowPath()); - return nullptr; - } - - return SlowPath(); -} - // This is a bit slow, but sometimes we need it. static bool ParentIsWrapperAnonBox(nsIFrame* aParent) { nsIFrame* maybeAnonBox = aParent; @@ -6315,288 +6157,7 @@ static bool ParentIsWrapperAnonBox(nsIFrame* aParent) { void nsCSSFrameConstructor::ContentAppended(nsIContent* aFirstNewContent, InsertionKind aInsertionKind) { - AUTO_PROFILER_LABEL_HOT("nsCSSFrameConstructor::ContentAppended", - LAYOUT_FrameConstruction); - AUTO_LAYOUT_PHASE_ENTRY_POINT(mPresShell->GetPresContext(), FrameC); - -#ifdef DEBUG - if (gNoisyContentUpdates) { - printf( - "nsCSSFrameConstructor::ContentAppended container=%p " - "first-child=%p lazy=%d\n", - aFirstNewContent->GetParent(), aFirstNewContent, - aInsertionKind == InsertionKind::Async); - if (gReallyNoisyContentUpdates && aFirstNewContent->GetParent()) { - aFirstNewContent->GetParent()->List(stdout, 0); - } - } - - for (nsIContent* child = aFirstNewContent; child; - child = child->GetNextSibling()) { - // XXX the GetContent() != child check is needed due to bug 135040. - // Remove it once that's fixed. - MOZ_ASSERT( - !child->GetPrimaryFrame() || - child->GetPrimaryFrame()->GetContent() != child, - "asked to construct a frame for a node that already has a frame"); - } -#endif - - LAYOUT_PHASE_TEMP_EXIT(); - InsertionPoint insertion = - GetRangeInsertionPoint(aFirstNewContent, nullptr, aInsertionKind); - nsContainerFrame*& parentFrame = insertion.mParentFrame; - LAYOUT_PHASE_TEMP_REENTER(); - if (!parentFrame) { - // We're punting on frame construction because there's no container frame. - // The Servo-backed style system handles this case like the lazy frame - // construction case, except when we're already constructing frames, in - // which case we shouldn't need to do anything else. - if (aInsertionKind == InsertionKind::Async) { - LazilyStyleNewChildRange(aFirstNewContent, nullptr); - } - return; - } - - if (aInsertionKind == InsertionKind::Async) { - ConstructLazily(CONTENTAPPEND, aFirstNewContent); - LazilyStyleNewChildRange(aFirstNewContent, nullptr); - return; - } - - LAYOUT_PHASE_TEMP_EXIT(); - if (MaybeRecreateForFrameset(parentFrame, aFirstNewContent, nullptr)) { - LAYOUT_PHASE_TEMP_REENTER(); - return; - } - LAYOUT_PHASE_TEMP_REENTER(); - - if (parentFrame->IsLeaf()) { - // Nothing to do here; we shouldn't be constructing kids of leaves - // Clear lazy bits so we don't try to construct again. - ClearLazyBits(aFirstNewContent, nullptr); - return; - } - - LAYOUT_PHASE_TEMP_EXIT(); - if (WipeInsertionParent(parentFrame)) { - LAYOUT_PHASE_TEMP_REENTER(); - return; - } - LAYOUT_PHASE_TEMP_REENTER(); - -#ifdef DEBUG - if (gNoisyContentUpdates && IsFramePartOfIBSplit(parentFrame)) { - printf("nsCSSFrameConstructor::ContentAppended: parentFrame="); - parentFrame->ListTag(stdout); - printf(" is ib-split\n"); - } -#endif - - // We should never get here with fieldsets, since they have - // multiple insertion points. - MOZ_ASSERT(!parentFrame->IsFieldSetFrame(), - "Parent frame should not be fieldset!"); - - nsIFrame* nextSibling = FindNextSiblingForAppend(insertion); - if (nextSibling) { - parentFrame = nextSibling->GetParent()->GetContentInsertionFrame(); - } else { - parentFrame = ::ContinuationToAppendTo(parentFrame); - } - - nsContainerFrame* containingBlock = GetFloatContainingBlock(parentFrame); - - // See if the containing block has :first-letter style applied. - const bool haveFirstLetterStyle = - containingBlock && HasFirstLetterStyle(containingBlock); - - const bool haveFirstLineStyle = - containingBlock && ShouldHaveFirstLineStyle(containingBlock->GetContent(), - containingBlock->Style()); - - if (haveFirstLetterStyle) { - AutoWeakFrame wf(nextSibling); - - // Before we get going, remove the current letter frames - RemoveLetterFrames(mPresShell, containingBlock); - - // Reget nextSibling, since we may have killed it. - // - // FIXME(emilio): This kinda sucks! :( - if (nextSibling && !wf) { - nextSibling = FindNextSiblingForAppend(insertion); - if (nextSibling) { - parentFrame = nextSibling->GetParent()->GetContentInsertionFrame(); - containingBlock = GetFloatContainingBlock(parentFrame); - } - } - } - - // Create some new frames - nsFrameConstructorState state( - mPresShell, GetAbsoluteContainingBlock(parentFrame, FIXED_POS), - GetAbsoluteContainingBlock(parentFrame, ABS_POS), containingBlock); - - if (mPresShell->GetPresContext()->IsPaginated()) { - // Because this function can be called outside frame construction, we need - // to set state.mAutoPageNameValue based on what the parent frame's auto - // value is. - // Calling this from outside the frame constructor can violate many of the - // expectations in AutoFrameConstructionPageName, and unlike during frame - // construction we already have an auto value from parentFrame, so we do - // not use AutoFrameConstructionPageName here. - state.mAutoPageNameValue = parentFrame->GetAutoPageValue(); -#ifdef DEBUG - parentFrame->mWasVisitedByAutoFrameConstructionPageName = true; -#endif - } - - LayoutFrameType frameType = parentFrame->Type(); - - RefPtr<ComputedStyle> parentStyle = - ResolveComputedStyle(insertion.mContainer); - FlattenedChildIterator iter(insertion.mContainer); - const bool haveNoShadowDOM = - !iter.ShadowDOMInvolved() || !iter.GetNextChild(); - - AutoFrameConstructionItemList items(this); - if (aFirstNewContent->GetPreviousSibling() && - GetParentType(frameType) == eTypeBlock && haveNoShadowDOM) { - // If there's a text node in the normal content list just before the new - // items, and it has no frame, make a frame construction item for it. If it - // doesn't need a frame, ConstructFramesFromItemList below won't give it - // one. No need to do all this if our parent type is not block, though, - // since WipeContainingBlock already handles that situation. - // - // Because we're appending, we don't need to worry about any text - // after the appended content; there can only be generated content - // (and bare text nodes are not generated). Native anonymous content - // generated by frames never participates in inline layout. - AddTextItemIfNeeded(state, *parentStyle, insertion, - aFirstNewContent->GetPreviousSibling(), items); - } - for (nsIContent* child = aFirstNewContent; child; - child = child->GetNextSibling()) { - AddFrameConstructionItems(state, child, false, *parentStyle, insertion, - items); - } - - nsIFrame* prevSibling = ::FindAppendPrevSibling(parentFrame, nextSibling); - - // Perform special check for diddling around with the frames in - // a ib-split inline frame. - // If we're appending before :after content, then we're not really - // appending, so let WipeContainingBlock know that. - LAYOUT_PHASE_TEMP_EXIT(); - if (WipeContainingBlock(state, containingBlock, parentFrame, items, true, - prevSibling)) { - LAYOUT_PHASE_TEMP_REENTER(); - return; - } - LAYOUT_PHASE_TEMP_REENTER(); - - // If the parent is a block frame, and we're not in a special case - // where frames can be moved around, determine if the list is for the - // start or end of the block. - if (parentFrame->IsBlockFrameOrSubclass() && !haveFirstLetterStyle && - !haveFirstLineStyle && !IsFramePartOfIBSplit(parentFrame)) { - items.SetLineBoundaryAtStart(!prevSibling || - !prevSibling->IsInlineOutside() || - prevSibling->IsBrFrame()); - // :after content can't be <br> so no need to check it - // - // FIXME(emilio): A display: contents sibling could! Write a test-case and - // fix. - items.SetLineBoundaryAtEnd(!nextSibling || !nextSibling->IsInlineOutside()); - } - // To suppress whitespace-only text frames, we have to verify that - // our container's DOM child list matches its flattened tree child list. - items.SetParentHasNoShadowDOM(haveNoShadowDOM); - - nsFrameConstructorSaveState floatSaveState; - state.MaybePushFloatContainingBlock(parentFrame, floatSaveState); - - nsFrameList frameList; - ConstructFramesFromItemList(state, items, parentFrame, - ParentIsWrapperAnonBox(parentFrame), frameList); - - for (nsIContent* child = aFirstNewContent; child; - child = child->GetNextSibling()) { - // Invalidate now instead of before the WipeContainingBlock call, just in - // case we do wipe; in that case we don't need to do this walk at all. - // XXXbz does that matter? Would it make more sense to save some virtual - // GetChildAt_Deprecated calls instead and do this during construction of - // our FrameConstructionItemList? - InvalidateCanvasIfNeeded(mPresShell, child); - } - - // If the container is a table and a caption was appended, it needs to be put - // in the table wrapper frame's additional child list. - nsFrameList captionList; - if (LayoutFrameType::Table == frameType) { - // Pull out the captions. Note that we don't want to do that as we go, - // because processing a single caption can add a whole bunch of things to - // the frame items due to pseudoframe processing. So we'd have to pull - // captions from a list anyway; might as well do that here. - // XXXbz this is no longer true; we could pull captions directly out of the - // FrameConstructionItemList now. - PullOutCaptionFrames(frameList, captionList); - } - - if (haveFirstLineStyle && parentFrame == containingBlock) { - // It's possible that some of the new frames go into a - // first-line frame. Look at them and see... - AppendFirstLineFrames(state, containingBlock->GetContent(), containingBlock, - frameList); - // That moved things into line frames as needed, reparenting their - // styles. Nothing else needs to be done. - } else if (parentFrame->Style()->IsInFirstLineSubtree()) { - // parentFrame might be inside a ::first-line frame. Check whether it is, - // and if so fix up our styles. - CheckForFirstLineInsertion(parentFrame, frameList); - CheckForFirstLineInsertion(parentFrame, captionList); - } - - // Notify the parent frame passing it the list of new frames - // Append the flowed frames to the principal child list; captions - // need special treatment - if (captionList.NotEmpty()) { // append the caption to the table wrapper - NS_ASSERTION(LayoutFrameType::Table == frameType, "how did that happen?"); - nsContainerFrame* outerTable = parentFrame->GetParent(); - captionList.ApplySetParent(outerTable); - AppendFrames(outerTable, FrameChildListID::Principal, - std::move(captionList)); - } - - LAYOUT_PHASE_TEMP_EXIT(); - if (MaybeRecreateForColumnSpan(state, parentFrame, frameList, prevSibling)) { - LAYOUT_PHASE_TEMP_REENTER(); - return; - } - LAYOUT_PHASE_TEMP_REENTER(); - - if (frameList.NotEmpty()) { // append the in-flow kids - AppendFramesToParent(state, parentFrame, frameList, prevSibling); - } - - // Recover first-letter frames - if (haveFirstLetterStyle) { - RecoverLetterFrames(containingBlock); - } - -#ifdef DEBUG - if (gReallyNoisyContentUpdates) { - printf("nsCSSFrameConstructor::ContentAppended: resulting frame model:\n"); - parentFrame->List(stdout); - } -#endif - -#ifdef ACCESSIBILITY - if (nsAccessibilityService* accService = GetAccService()) { - accService->ContentRangeInserted(mPresShell, aFirstNewContent, nullptr); - } -#endif + return ContentRangeInserted(aFirstNewContent, nullptr, aInsertionKind); } void nsCSSFrameConstructor::ContentInserted(nsIContent* aChild, @@ -6604,25 +6165,36 @@ void nsCSSFrameConstructor::ContentInserted(nsIContent* aChild, ContentRangeInserted(aChild, aChild->GetNextSibling(), aInsertionKind); } +// It's very likely that we don't have any existing captions (because we only +// render one of them, see bug 144517). We also don't fragment table captions. +// So, prefer just walking the existing captions and searching it, rather than +// adding a special GetInsertionPrevSibling() version that skips everything but +// table captions. +static nsIFrame* FindCaptionPrevSibling(nsTableWrapperFrame* aTable, + nsIContent* aCaptionContent) { + nsIFrame* prevSibling = aTable->InnerTableFrame(); + if (nsIFrame* firstCaption = prevSibling->GetNextSibling()) { + nsContentUtils::NodeIndexCache cache; + for (auto* caption = firstCaption; caption; + caption = caption->GetNextSibling()) { + if (nsContentUtils::CompareTreePosition<TreeKind::Flat>( + caption->GetContent(), aCaptionContent, nullptr, &cache) >= 0) { + break; + } + prevSibling = caption; + } + } + return prevSibling; +} + // ContentRangeInserted handles creating frames for a range of nodes that -// aren't at the end of their childlist. ContentRangeInserted isn't a real +// might not be at the end of their childlist. ContentRangeInserted isn't a real // content notification, but rather it handles regular ContentInserted calls // for a single node as well as the lazy construction of frames for a range of // nodes when called from CreateNeededFrames. For a range of nodes to be // suitable to have its frames constructed all at once they must meet the same // conditions that ContentAppended imposes (GetRangeInsertionPoint checks -// these), plus more. Namely when finding the insertion prevsibling we must not -// need to consult something specific to any one node in the range, so that the -// insertion prevsibling would be the same for each node in the range. So we -// pass the first node in the range to GetInsertionPrevSibling, and if -// IsValidSibling (the only place GetInsertionPrevSibling might look at the -// passed in node itself) needs to resolve style on the node we record this and -// return that this range needs to be split up and inserted separately. -// Table captions require special handling, as we need to determine where to -// insert them in the table wrapper frame's principal child list while skipping -// any nodes in the range being inserted. This is because when we process the -// caption frames, the other nodes have already had their frames constructed, -// but those frames have not yet been inserted into the frame tree. +// these), plus more. void nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aStartChild, nsIContent* aEndChild, InsertionKind aInsertionKind) { @@ -6652,18 +6224,14 @@ void nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aStartChild, child = child->GetNextSibling()) { // XXX the GetContent() != child check is needed due to bug 135040. // Remove it once that's fixed. - NS_ASSERTION( + MOZ_ASSERT( !child->GetPrimaryFrame() || child->GetPrimaryFrame()->GetContent() != child, "asked to construct a frame for a node that already has a frame"); } #endif - bool isSingleInsert = (aStartChild->GetNextSibling() == aEndChild); - NS_ASSERTION(isSingleInsert || aInsertionKind == InsertionKind::Sync, - "range insert shouldn't be lazy"); - NS_ASSERTION(isSingleInsert || aEndChild, - "range should not include all nodes after aStartChild"); + const bool isSingleInsert = (aStartChild->GetNextSibling() == aEndChild); // If we have a null parent, then this must be the document element being // inserted, or some other child of the document in the DOM (might be a PI, @@ -6732,23 +6300,14 @@ void nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aStartChild, } if (aInsertionKind == InsertionKind::Async) { - ConstructLazily(CONTENTINSERT, aStartChild); + ConstructLazily(aStartChild, aEndChild); LazilyStyleNewChildRange(aStartChild, aEndChild); return; } - bool isAppend, isRangeInsertSafe; - nsIFrame* prevSibling = GetInsertionPrevSibling( - &insertion, aStartChild, &isAppend, &isRangeInsertSafe); - - // check if range insert is safe - if (!isSingleInsert && !isRangeInsertSafe) { - // must fall back to a single ContertInserted for each child in the range - LAYOUT_PHASE_TEMP_EXIT(); - IssueSingleInsertNofications(aStartChild, aEndChild, InsertionKind::Sync); - LAYOUT_PHASE_TEMP_REENTER(); - return; - } + bool isAppend; + nsIFrame* prevSibling = + GetInsertionPrevSibling(&insertion, aStartChild, &isAppend); LayoutFrameType frameType = insertion.mParentFrame->Type(); LAYOUT_PHASE_TEMP_EXIT(); @@ -6759,32 +6318,6 @@ void nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aStartChild, } LAYOUT_PHASE_TEMP_REENTER(); - // We should only get here with fieldsets when doing a single insert, because - // fieldsets have multiple insertion points. - NS_ASSERTION(isSingleInsert || frameType != LayoutFrameType::FieldSet, - "Unexpected parent"); - // Note that this check is insufficient if aStartChild is not a legend with - // display::contents that contains a legend. We'll catch that case in - // WipeContainingBlock. (That code would also catch this case, but handling - // this early is slightly faster.) - // XXXmats we should be able to optimize this when the fieldset doesn't - // currently have a rendered legend. ContentRangeInserted needs to be fixed - // to use the inner frame as the content insertion frame in that case. - if (GetFieldSetFrameFor(insertion.mParentFrame) && - aStartChild->NodeInfo()->NameAtom() == nsGkAtoms::legend) { - // Just reframe the parent, since figuring out whether this - // should be the new legend and then handling it is too complex. - // We could do a little better here --- check if the fieldset already - // has a legend which occurs earlier in its child list than this node, - // and if so, proceed. But we'd have to extend nsFieldSetFrame - // to locate this legend in the inserted frames and extract it. - LAYOUT_PHASE_TEMP_EXIT(); - RecreateFramesForContent(insertion.mParentFrame->GetContent(), - InsertionKind::Async); - LAYOUT_PHASE_TEMP_REENTER(); - return; - } - // Don't construct kids of leaves if (insertion.mParentFrame->IsLeaf()) { // Clear lazy bits so we don't try to construct again. @@ -6854,11 +6387,10 @@ void nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aStartChild, // Removing the letterframes messes around with the frame tree, removing // and creating frames. We need to reget our prevsibling, parent frame, // etc. - prevSibling = GetInsertionPrevSibling(&insertion, aStartChild, &isAppend, - &isRangeInsertSafe); + prevSibling = GetInsertionPrevSibling(&insertion, aStartChild, &isAppend); // Need check whether a range insert is still safe. - if (!isSingleInsert && !isRangeInsertSafe) { + if (!isSingleInsert) { // Need to recover the letter frames first. RecoverLetterFrames(state.mFloatedList.mContainingBlock); @@ -6938,16 +6470,12 @@ void nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aStartChild, aStartChild->GetPreviousSibling(), items); } - if (isSingleInsert) { - AddFrameConstructionItems(state, aStartChild, - aStartChild->IsRootOfNativeAnonymousSubtree(), + const bool suppressWhiteSpaceOptimizations = + isSingleInsert && aStartChild->IsRootOfNativeAnonymousSubtree(); + for (nsIContent* child = aStartChild; child != aEndChild; + child = child->GetNextSibling()) { + AddFrameConstructionItems(state, child, suppressWhiteSpaceOptimizations, *parentStyle, insertion, items); - } else { - for (nsIContent* child = aStartChild; child != aEndChild; - child = child->GetNextSibling()) { - AddFrameConstructionItems(state, child, false, *parentStyle, insertion, - items); - } } if (aEndChild && parentType == eTypeBlock && haveNoShadowDOM) { @@ -6959,10 +6487,9 @@ void nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aStartChild, AddTextItemIfNeeded(state, *parentStyle, insertion, aEndChild, items); } - // Perform special check for diddling around with the frames in - // a special inline frame. - // If we're appending before :after content, then we're not really - // appending, so let WipeContainingBlock know that. + // Perform special check for diddling around with the frames in a special + // inline frame. If we're appending before :after content, then we're not + // really appending, so let WipeContainingBlock know that. LAYOUT_PHASE_TEMP_EXIT(); if (WipeContainingBlock(state, containingBlock, insertion.mParentFrame, items, isAppend, prevSibling)) { @@ -6971,6 +6498,25 @@ void nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aStartChild, } LAYOUT_PHASE_TEMP_REENTER(); + // If the parent is a block frame, and we're not in a special case + // where frames can be moved around, determine if the list is for the + // start or end of the block. + if (insertion.mParentFrame->IsBlockFrameOrSubclass() && + !haveFirstLetterStyle && !haveFirstLineStyle && + !IsFramePartOfIBSplit(insertion.mParentFrame)) { + items.SetLineBoundaryAtStart(!prevSibling || + !prevSibling->IsInlineOutside() || + prevSibling->IsBrFrame()); + auto* nextSibling = + ::GetInsertNextSibling(insertion.mParentFrame, prevSibling); + items.SetLineBoundaryAtEnd(!nextSibling || + !nextSibling->IsInlineOutside() || + nextSibling->IsBrFrame()); + } + // To suppress whitespace-only text frames, we have to verify that + // our container's DOM child list matches its flattened tree child list. + items.SetParentHasNoShadowDOM(haveNoShadowDOM); + nsFrameConstructorSaveState floatSaveState; state.MaybePushFloatContainingBlock(insertion.mParentFrame, floatSaveState); @@ -7034,29 +6580,9 @@ void nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aStartChild, // We need to determine where to put the caption items; start with the // the parent frame that has already been determined and get the insertion // prevsibling of the first caption item. - bool captionIsAppend; - nsIFrame* captionPrevSibling = nullptr; - - // aIsRangeInsertSafe is ignored on purpose because it is irrelevant here. - bool ignored; - InsertionPoint captionInsertion = insertion; - if (isSingleInsert) { - captionPrevSibling = GetInsertionPrevSibling( - &captionInsertion, aStartChild, &captionIsAppend, &ignored); - } else { - nsIContent* firstCaption = captionList.FirstChild()->GetContent(); - // It is very important here that we skip the children in - // [aStartChild,aEndChild) when looking for a - // prevsibling. - captionPrevSibling = GetInsertionPrevSibling( - &captionInsertion, firstCaption, &captionIsAppend, &ignored, - aStartChild, aEndChild); - } - - nsContainerFrame* outerTable = - captionInsertion.mParentFrame->IsTableFrame() - ? captionInsertion.mParentFrame->GetParent() - : captionInsertion.mParentFrame; + nsContainerFrame* outerTable = insertion.mParentFrame->IsTableFrame() + ? insertion.mParentFrame->GetParent() + : insertion.mParentFrame; // If the parent is not a table wrapper frame we will try to add frames // to a named child list that the parent does not honor and the frames @@ -7065,17 +6591,11 @@ void nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aStartChild, "Pseudo frame construction failure; " "a caption can be only a child of a table wrapper frame"); - // If the parent of our current prevSibling is different from the frame - // we'll actually use as the parent, then the calculated insertion - // point is now invalid (bug 341382). Insert right after the table frame - // instead. - if (!captionPrevSibling || captionPrevSibling->GetParent() != outerTable) { - captionPrevSibling = - static_cast<nsTableWrapperFrame*>(outerTable)->InnerTableFrame(); - } - + nsIFrame* captionPrevSibling = + FindCaptionPrevSibling(static_cast<nsTableWrapperFrame*>(outerTable), + captionList.FirstChild()->GetContent()); captionList.ApplySetParent(outerTable); - if (captionIsAppend) { + if (!captionPrevSibling->GetNextSibling()) { AppendFrames(outerTable, FrameChildListID::Principal, std::move(captionList)); } else { @@ -10834,8 +10354,8 @@ bool nsCSSFrameConstructor::MaybeRecreateForColumnSpan( } MOZ_ASSERT(!IsFramePartOfIBSplit(aParentFrame), - "We should have wiped aParentFrame in WipeContainingBlock if it's " - "part of IB split!"); + "We should have wiped aParentFrame in " + "WipeContainingBlock if it's part of IB split!"); nsIFrame* nextSibling = ::GetInsertNextSibling(aParentFrame, aPrevSibling); if (!nextSibling && IsLastContinuationForColumnContent(aParentFrame)) { @@ -11472,12 +10992,9 @@ bool nsCSSFrameConstructor::WipeContainingBlock( okToDrop = (nextSibling && !IsTablePseudo(nextSibling)) || (!nextSibling && !IsTablePseudo(aFrame)); - } -#ifdef DEBUG - else { + } else { NS_ASSERTION(!IsTablePseudo(aFrame), "How did that happen?"); } -#endif } else { okToDrop = (spaceEndIter.item().DesiredParentType() == parentType); } diff --git a/layout/base/nsCSSFrameConstructor.h b/layout/base/nsCSSFrameConstructor.h @@ -17,7 +17,6 @@ #include "mozilla/ContainStyleScopeManager.h" #include "mozilla/FunctionRef.h" #include "mozilla/LinkedList.h" -#include "mozilla/Maybe.h" #include "mozilla/ScrollStyles.h" #include "mozilla/UniquePtr.h" #include "nsCOMPtr.h" @@ -100,11 +99,10 @@ class nsCSSFrameConstructor final : public nsFrameManager { mozilla::ViewportFrame* ConstructRootFrame(); private: - enum Operation { CONTENTAPPEND, CONTENTINSERT }; - // aChild is the child being inserted for inserts, and the first - // child being appended for appends. - void ConstructLazily(Operation aOperation, nsIContent* aChild); + // child being appended for appends. All the nodes in the range are + // guaranteed to have the same flat tree parent. + void ConstructLazily(nsIContent* aStartChild, nsIContent* aEndChild); #ifdef DEBUG void CheckBitsForLazyFrameConstruction(nsIContent* aParent); @@ -140,12 +138,6 @@ class nsCSSFrameConstructor final : public nsFrameManager { * It's undefined if mParentFrame is null. */ nsIContent* mContainer; - - /** - * Whether it is required to insert children one-by-one instead of as a - * range. - */ - bool IsMultiple() const; }; /** @@ -1904,10 +1896,10 @@ class nsCSSFrameConstructor final : public nsFrameManager { // rebuild the entire subtree when we insert or append new content under // aFrame. // - // This is similar to WipeContainingBlock(), but is called before constructing - // any frame construction items. Any container frames which need reframing - // regardless of the content inserted or appended can add a check in this - // method. + // This is similar to WipeContainingBlock(), but is called + // before constructing any frame construction items. Any container frames + // which need reframing regardless of the content inserted or appended can add + // a check in this method. // // @return true if we reconstructed the insertion parent frame; false // otherwise @@ -1917,12 +1909,9 @@ class nsCSSFrameConstructor final : public nsFrameManager { // because we're doing something like adding block kids to an inline frame // (and therefore need an {ib} split). aPrevSibling must be correct, even in // aIsAppend cases. Passing aIsAppend false even when an append is happening - // is ok in terms of correctness, but can lead to unnecessary reframing. If - // aIsAppend is true, then the caller MUST call - // nsCSSFrameConstructor::AppendFramesToParent (as opposed to - // nsFrameManager::InsertFrames directly) to add the new frames. - // @return true if we reconstructed the containing block, false - // otherwise + // is ok in terms of correctness, but can lead to unnecessary reframing. + // + // @return true if we reconstructed the containing block, false otherwise. bool WipeContainingBlock(nsFrameConstructorState& aState, nsIFrame* aContainingBlock, nsIFrame* aFrame, FrameConstructionItemList& aItems, bool aIsAppend, @@ -2027,14 +2016,6 @@ class nsCSSFrameConstructor final : public nsFrameManager { void CheckForFirstLineInsertion(nsIFrame* aParentFrame, nsFrameList& aFrameList); - /** - * Find the next frame for appending to a given insertion point. - * - * We're appending, so this is almost always null, except for a few edge - * cases. - */ - nsIFrame* FindNextSiblingForAppend(const InsertionPoint&); - // The direction in which we should look for siblings. enum class SiblingDirection { Forward, @@ -2051,65 +2032,34 @@ class nsCSSFrameConstructor final : public nsFrameManager { * * @param aIter should be positioned such that aIter.GetPreviousChild() * is the first content to search for frames - * @param aTargetContentDisplay the CSS display enum for the content aIter - * points to if already known. It will be filled in if needed. */ template <SiblingDirection> - nsIFrame* FindSibling( - const mozilla::dom::FlattenedChildIterator& aIter, - mozilla::Maybe<mozilla::StyleDisplay>& aTargetContentDisplay); + nsIFrame* FindSibling(const mozilla::dom::FlattenedChildIterator& aIter); // Helper for the implementation of FindSibling. // // Beware that this function does mutate the iterator. template <SiblingDirection> - nsIFrame* FindSiblingInternal( - mozilla::dom::FlattenedChildIterator&, nsIContent* aTargetContent, - mozilla::Maybe<mozilla::StyleDisplay>& aTargetContentDisplay); + nsIFrame* FindSiblingInternal(mozilla::dom::FlattenedChildIterator&); // An alias of FindSibling<SiblingDirection::Forward>. - nsIFrame* FindNextSibling( - const mozilla::dom::FlattenedChildIterator& aIter, - mozilla::Maybe<mozilla::StyleDisplay>& aTargetContentDisplay); - // An alias of FindSibling<SiblingDirection::Backwards>. + nsIFrame* FindNextSibling(const mozilla::dom::FlattenedChildIterator& aIter); + // An alias of FindSibling<SiblingDirection::Backward>. nsIFrame* FindPreviousSibling( - const mozilla::dom::FlattenedChildIterator& aIter, - mozilla::Maybe<mozilla::StyleDisplay>& aTargetContentDisplay); - - // Given a potential first-continuation sibling frame for aTargetContent, - // verify that it is an actual valid sibling for it, and return the - // appropriate continuation the new frame for aTargetContent should be - // inserted next to. - nsIFrame* AdjustSiblingFrame( - nsIFrame* aSibling, nsIContent* aTargetContent, - mozilla::Maybe<mozilla::StyleDisplay>& aTargetContentDisplay, - SiblingDirection aDirection); + const mozilla::dom::FlattenedChildIterator& aIter); + + // Given a potential first-continuation sibling frame, return the + // appropriate continuation the new frame should be inserted next to. + nsIFrame* AdjustSiblingFrame(nsIFrame* aSibling, SiblingDirection); // Find the right previous sibling for an insertion. This also updates the // parent frame to point to the correct continuation of the parent frame to // use, and returns whether this insertion is to be treated as an append. // aChild is the child being inserted. - // aIsRangeInsertSafe returns whether it is safe to do a range insert with - // aChild being the first child in the range. It is the callers' - // responsibility to check whether a range insert is safe with regards to - // fieldsets. - // The skip parameters are used to ignore a range of children when looking - // for a sibling. All nodes starting from aStartSkipChild and up to but not - // including aEndSkipChild will be skipped over when looking for sibling - // frames. Skipping a range can deal with shadow DOM, but not when there are - // multiple insertion points. + // It is the callers' responsibility to check whether a range insert is safe + // with regards to fieldsets / tables. nsIFrame* GetInsertionPrevSibling(InsertionPoint* aInsertion, // inout - nsIContent* aChild, bool* aIsAppend, - bool* aIsRangeInsertSafe, - nsIContent* aStartSkipChild = nullptr, - nsIContent* aEndSkipChild = nullptr); - - // see if aContent and aSibling are legitimate siblings due to restrictions - // imposed by table columns - // XXXbz this code is generally wrong, since the frame for aContent - // may be constructed based on tag, not based on aDisplay! - bool IsValidSibling(nsIFrame* aSibling, nsIContent* aContent, - mozilla::Maybe<mozilla::StyleDisplay>& aDisplay); + nsIContent* aChild, bool* aIsAppend); void QuotesDirty(); void CountersDirty(); diff --git a/layout/forms/nsFieldSetFrame.h b/layout/forms/nsFieldSetFrame.h @@ -87,6 +87,13 @@ class nsFieldSetFrame final : public nsContainerFrame { */ nsContainerFrame* GetInner() const; + nsContainerFrame* GetContentInsertionFrame() override { + if (auto* inner = GetInner()) { + return inner->GetContentInsertionFrame(); + } + return this; + } + /** * Return the frame that represents the rendered legend if any. * https://html.spec.whatwg.org/multipage/rendering.html#rendered-legend diff --git a/layout/generic/nsFlexContainerFrame.cpp b/layout/generic/nsFlexContainerFrame.cpp @@ -5195,27 +5195,12 @@ void nsFlexContainerFrame::UpdateFlexLineAndItemInfo( nsFlexContainerFrame* nsFlexContainerFrame::GetFlexFrameWithComputedInfo( nsIFrame* aFrame) { // Prepare a lambda function that we may need to call multiple times. - auto GetFlexContainerFrame = [](nsIFrame* aFrame) { - // Return the aFrame's content insertion frame, iff it is - // a flex container frame. - nsFlexContainerFrame* flexFrame = nullptr; - - if (aFrame) { - nsIFrame* inner = aFrame; - if (MOZ_UNLIKELY(aFrame->IsFieldSetFrame())) { - inner = static_cast<nsFieldSetFrame*>(aFrame)->GetInner(); - } - // Since "Get" methods like GetInner and GetContentInsertionFrame can - // return null, we check the return values before dereferencing. Our - // calling pattern makes this unlikely, but we're being careful. - nsIFrame* insertionFrame = - inner ? inner->GetContentInsertionFrame() : nullptr; - nsIFrame* possibleFlexFrame = insertionFrame ? insertionFrame : aFrame; - flexFrame = possibleFlexFrame->IsFlexContainerFrame() - ? static_cast<nsFlexContainerFrame*>(possibleFlexFrame) - : nullptr; + auto GetFlexContainerFrame = [](nsIFrame* aFrame) -> nsFlexContainerFrame* { + // Return the aFrame's content insertion frame, iff it is a flex container. + if (!aFrame) { + return nullptr; } - return flexFrame; + return do_QueryFrame(aFrame->GetContentInsertionFrame()); }; nsFlexContainerFrame* flexFrame = GetFlexContainerFrame(aFrame); diff --git a/layout/inspector/InspectorUtils.cpp b/layout/inspector/InspectorUtils.cpp @@ -1070,12 +1070,6 @@ bool InspectorUtils::IsBlockContainer(GlobalObject&, Element& aElement) { if (!frame) { return false; } - - // For fieldset elements, we need to check the inner frame. - if (nsFieldSetFrame* fieldsetFrame = do_QueryFrame(frame)) { - frame = fieldsetFrame->GetInner(); - } - if (frame->IsBlockFrameOrSubclass()) { return true; } @@ -1092,7 +1086,6 @@ bool InspectorUtils::IsBlockContainer(GlobalObject&, Element& aElement) { return true; } } - return false; } diff --git a/testing/web-platform/tests/css/css-tables/table-caption-after-crash.html b/testing/web-platform/tests/css/css-tables/table-caption-after-crash.html @@ -0,0 +1,10 @@ +<!doctype html> +<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=2008148"> +<style> +*::after, .a:only-of-type { + content: counters(ct1, '.', disc); + display: table-caption; +} +</style> +<code></code> +<object class="a">