From 11bec6d3bcb1ae265b50daefee06eaafe136060d Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 21 Feb 2012 18:01:40 -0500 Subject: [PATCH] Bug 728516. Refactor frame construction for things that want to wrap kids in a block. r=roc --- layout/base/nsCSSFrameConstructor.cpp | 265 ++++++------------ layout/base/nsCSSFrameConstructor.h | 30 +- .../forms/button-first-letter-1-noref.html | 5 + .../forms/button-first-letter-1-ref.html | 7 + .../reftests/forms/button-first-letter-1.html | 6 + layout/reftests/forms/reftest.list | 3 + 6 files changed, 113 insertions(+), 203 deletions(-) create mode 100644 layout/reftests/forms/button-first-letter-1-noref.html create mode 100644 layout/reftests/forms/button-first-letter-1-ref.html create mode 100644 layout/reftests/forms/button-first-letter-1.html diff --git a/layout/base/nsCSSFrameConstructor.cpp b/layout/base/nsCSSFrameConstructor.cpp index 4931301ca69e..a6f16adda268 100644 --- a/layout/base/nsCSSFrameConstructor.cpp +++ b/layout/base/nsCSSFrameConstructor.cpp @@ -373,8 +373,11 @@ GetFieldSetBlockFrame(nsIFrame* aFieldsetFrame) return firstChild && firstChild->GetNextSibling() ? firstChild->GetNextSibling() : firstChild; } -#define FCDATA_DECL(_flags, _func) \ - { _flags, { (FrameCreationFunc)_func } } +#define FCDATA_DECL(_flags, _func) \ + { _flags, { (FrameCreationFunc)_func }, nsnull, nsnull } +#define FCDATA_WITH_WRAPPING_BLOCK(_flags, _func, _anon_box) \ + { _flags | FCDATA_CREATE_BLOCK_WRAPPER_FOR_ALL_KIDS, \ + { (FrameCreationFunc)_func }, nsnull, &_anon_box } //---------------------------------------------------------------------- @@ -2929,120 +2932,6 @@ ClearLazyBits(nsIContent* aStartContent, nsIContent* aEndContent) } } -nsresult -nsCSSFrameConstructor::ConstructButtonFrame(nsFrameConstructorState& aState, - FrameConstructionItem& aItem, - nsIFrame* aParentFrame, - const nsStyleDisplay* aStyleDisplay, - nsFrameItems& aFrameItems, - nsIFrame** aNewFrame) -{ - *aNewFrame = nsnull; - nsIFrame* buttonFrame = nsnull; - nsIContent* const content = aItem.mContent; - nsStyleContext* const styleContext = aItem.mStyleContext; - - if (nsGkAtoms::button == aItem.mTag) { - buttonFrame = NS_NewHTMLButtonControlFrame(mPresShell, styleContext); - } - else { - buttonFrame = NS_NewGfxButtonControlFrame(mPresShell, styleContext); - } - if (NS_UNLIKELY(!buttonFrame)) { - return NS_ERROR_OUT_OF_MEMORY; - } - // Initialize the button frame - nsresult rv = InitAndRestoreFrame(aState, content, - aState.GetGeometricParent(aStyleDisplay, aParentFrame), - nsnull, buttonFrame); - if (NS_FAILED(rv)) { - buttonFrame->Destroy(); - return rv; - } - - nsRefPtr innerBlockContext; - innerBlockContext = - mPresShell->StyleSet()->ResolveAnonymousBoxStyle(nsCSSAnonBoxes::buttonContent, - styleContext); - - nsIFrame* blockFrame = NS_NewBlockFrame(mPresShell, innerBlockContext, - NS_BLOCK_FLOAT_MGR); - - if (NS_UNLIKELY(!blockFrame)) { - buttonFrame->Destroy(); - return NS_ERROR_OUT_OF_MEMORY; - } - rv = InitAndRestoreFrame(aState, content, buttonFrame, nsnull, blockFrame); - if (NS_FAILED(rv)) { - blockFrame->Destroy(); - buttonFrame->Destroy(); - return rv; - } - - rv = aState.AddChild(buttonFrame, aFrameItems, content, styleContext, - aParentFrame); - if (NS_FAILED(rv)) { - blockFrame->Destroy(); - buttonFrame->Destroy(); - return rv; - } - - bool isLeaf = buttonFrame->IsLeaf(); -#ifdef DEBUG - // Make sure that we're an anonymous content creator exactly when we're a - // leaf - nsIAnonymousContentCreator* creator = do_QueryFrame(buttonFrame); - NS_ASSERTION(!creator == !isLeaf, - "Should be creator exactly when we're a leaf"); -#endif - - if (!isLeaf) { - // Process children - nsFrameConstructorSaveState absoluteSaveState; - nsFrameItems childItems; - - if (aStyleDisplay->IsPositioned()) { - aState.PushAbsoluteContainingBlock(buttonFrame, absoluteSaveState); - } - -#ifdef DEBUG - // Make sure that anonymous child creation will have no effect in this case - nsIAnonymousContentCreator* creator = do_QueryFrame(blockFrame); - NS_ASSERTION(!creator, "Shouldn't be an anonymous content creator!"); -#endif - - rv = ProcessChildren(aState, content, styleContext, blockFrame, true, - childItems, aStyleDisplay->IsBlockInside(), - aItem.mPendingBinding); - if (NS_FAILED(rv)) return rv; - - // Set the areas frame's initial child lists - blockFrame->SetInitialChildList(kPrincipalList, childItems); - } - - SetInitialSingleChild(buttonFrame, blockFrame); - - if (isLeaf) { - ClearLazyBits(content->GetFirstChild(), nsnull); - - nsFrameItems anonymousChildItems; - // if there are any anonymous children create frames for them. Note that - // we're doing this using a different parent frame from the one we pass to - // ProcessChildren! - CreateAnonymousFrames(aState, content, buttonFrame, aItem.mPendingBinding, - anonymousChildItems); - if (anonymousChildItems.NotEmpty()) { - // the anonymous content is already parented to the area frame - AppendFrames(blockFrame, kPrincipalList, anonymousChildItems); - } - } - - // our new button frame returned is the top frame. - *aNewFrame = buttonFrame; - - return NS_OK; -} - nsresult nsCSSFrameConstructor::ConstructSelectFrame(nsFrameConstructorState& aState, FrameConstructionItem& aItem, @@ -3325,8 +3214,8 @@ FindAncestorWithGeneratedContentPseudo(nsIFrame* aFrame) } #define SIMPLE_FCDATA(_func) FCDATA_DECL(0, _func) -#define FULL_CTOR_FCDATA(_flags, _func) \ - { _flags | FCDATA_FUNC_IS_FULL_CTOR, { nsnull }, _func } +#define FULL_CTOR_FCDATA(_flags, _func) \ + { _flags | FCDATA_FUNC_IS_FULL_CTOR, { nsnull }, _func, nsnull } /* static */ const nsCSSFrameConstructor::FrameConstructionData* @@ -3523,7 +3412,10 @@ nsCSSFrameConstructor::FindHTMLData(Element* aElement, FCDATA_DECL(FCDATA_ALLOW_BLOCK_STYLES, NS_NewLegendFrame) }, SIMPLE_TAG_CREATE(frameset, NS_NewHTMLFramesetFrame), SIMPLE_TAG_CREATE(iframe, NS_NewSubDocumentFrame), - COMPLEX_TAG_CREATE(button, &nsCSSFrameConstructor::ConstructButtonFrame), + { &nsGkAtoms::button, + FCDATA_WITH_WRAPPING_BLOCK(FCDATA_ALLOW_BLOCK_STYLES, + NS_NewHTMLButtonControlFrame, + nsCSSAnonBoxes::buttonContent) }, SIMPLE_TAG_CHAIN(canvas, nsCSSFrameConstructor::FindCanvasData), #if defined(MOZ_MEDIA) SIMPLE_TAG_CREATE(video, NS_NewHTMLVideoFrame), @@ -3580,12 +3472,15 @@ nsCSSFrameConstructor::FindInputData(Element* aElement, SIMPLE_INT_CREATE(NS_FORM_INPUT_TEL, NS_NewTextControlFrame), SIMPLE_INT_CREATE(NS_FORM_INPUT_URL, NS_NewTextControlFrame), SIMPLE_INT_CREATE(NS_FORM_INPUT_PASSWORD, NS_NewTextControlFrame), - COMPLEX_INT_CREATE(NS_FORM_INPUT_SUBMIT, - &nsCSSFrameConstructor::ConstructButtonFrame), - COMPLEX_INT_CREATE(NS_FORM_INPUT_RESET, - &nsCSSFrameConstructor::ConstructButtonFrame), - COMPLEX_INT_CREATE(NS_FORM_INPUT_BUTTON, - &nsCSSFrameConstructor::ConstructButtonFrame) + { NS_FORM_INPUT_SUBMIT, + FCDATA_WITH_WRAPPING_BLOCK(0, NS_NewGfxButtonControlFrame, + nsCSSAnonBoxes::buttonContent) }, + { NS_FORM_INPUT_RESET, + FCDATA_WITH_WRAPPING_BLOCK(0, NS_NewGfxButtonControlFrame, + nsCSSAnonBoxes::buttonContent) }, + { NS_FORM_INPUT_BUTTON, + FCDATA_WITH_WRAPPING_BLOCK(0, NS_NewGfxButtonControlFrame, + nsCSSAnonBoxes::buttonContent) } // Keeping hidden inputs out of here on purpose for so they get frames by // display (in practice, none). }; @@ -3675,6 +3570,10 @@ nsCSSFrameConstructor::ConstructFrameFromItemInternal(FrameConstructionItem& aIt CHECK_ONLY_ONE_BIT(FCDATA_FUNC_IS_FULL_CTOR, FCDATA_DISALLOW_GENERATED_CONTENT); CHECK_ONLY_ONE_BIT(FCDATA_FUNC_IS_FULL_CTOR, FCDATA_ALLOW_BLOCK_STYLES); + CHECK_ONLY_ONE_BIT(FCDATA_FUNC_IS_FULL_CTOR, + FCDATA_CREATE_BLOCK_WRAPPER_FOR_ALL_KIDS); + CHECK_ONLY_ONE_BIT(FCDATA_WRAP_KIDS_IN_BLOCKS, + FCDATA_CREATE_BLOCK_WRAPPER_FOR_ALL_KIDS); #undef CHECK_ONLY_ONE_BIT NS_ASSERTION(!(bits & FCDATA_FORCED_NON_SCROLLABLE_BLOCK) || ((bits & FCDATA_FUNC_IS_FULL_CTOR) && @@ -3748,6 +3647,45 @@ nsCSSFrameConstructor::ConstructFrameFromItemInternal(FrameConstructionItem& aIt // primary frame). primaryFrame = frameToAddToList; + // If we need to create a block formatting context to wrap our + // kids, do it now. + const nsStyleDisplay* maybeAbsoluteContainingBlockDisplay = display; + nsIFrame* maybeAbsoluteContainingBlock = newFrame; + nsIFrame* possiblyLeafFrame = newFrame; + if (bits & FCDATA_CREATE_BLOCK_WRAPPER_FOR_ALL_KIDS) { + nsRefPtr blockContext; + blockContext = + mPresShell->StyleSet()->ResolveAnonymousBoxStyle(*data->mAnonBoxPseudo, + styleContext); + nsIFrame* blockFrame = + NS_NewBlockFormattingContext(mPresShell, blockContext); + if (NS_UNLIKELY(!blockFrame)) { + primaryFrame->Destroy(); + return NS_ERROR_OUT_OF_MEMORY; + } + + rv = InitAndRestoreFrame(aState, content, newFrame, nsnull, blockFrame); + if (NS_FAILED(rv)) { + blockFrame->Destroy(); + primaryFrame->Destroy(); + return rv; + } + + SetInitialSingleChild(newFrame, blockFrame); + + // Now figure out whether newFrame or blockFrame should be the + // absolute container. It should be the latter if it's + // positioned, otherwise the former. + const nsStyleDisplay* blockDisplay = blockContext->GetStyleDisplay(); + if (blockDisplay->IsPositioned()) { + maybeAbsoluteContainingBlockDisplay = blockDisplay; + maybeAbsoluteContainingBlock = blockFrame; + } + + // Our kids should go into the blockFrame + newFrame = blockFrame; + } + rv = aState.AddChild(frameToAddToList, aFrameItems, content, styleContext, aParentFrame, allowOutOfFlow, allowOutOfFlow, isPopup); if (NS_FAILED(rv)) { @@ -3773,8 +3711,10 @@ nsCSSFrameConstructor::ConstructFrameFromItemInternal(FrameConstructionItem& aIt if (bits & FCDATA_FORCE_NULL_ABSPOS_CONTAINER) { aState.PushAbsoluteContainingBlock(nsnull, absoluteSaveState); - } else if (!(bits & FCDATA_SKIP_ABSPOS_PUSH) && display->IsPositioned()) { - aState.PushAbsoluteContainingBlock(newFrame, absoluteSaveState); + } else if (!(bits & FCDATA_SKIP_ABSPOS_PUSH) && + maybeAbsoluteContainingBlockDisplay->IsPositioned()) { + aState.PushAbsoluteContainingBlock(maybeAbsoluteContainingBlock, + absoluteSaveState); } if (bits & FCDATA_USE_CHILD_ITEMS) { @@ -3786,7 +3726,7 @@ nsCSSFrameConstructor::ConstructFrameFromItemInternal(FrameConstructionItem& aIt !(bits & FCDATA_DISALLOW_GENERATED_CONTENT), childItems, (bits & FCDATA_ALLOW_BLOCK_STYLES) != 0, - aItem.mPendingBinding); + aItem.mPendingBinding, possiblyLeafFrame); } #ifdef MOZ_XUL @@ -4861,8 +4801,9 @@ nsCSSFrameConstructor::FindSVGData(Element* aElement, SIMPLE_SVG_CREATE(defs, NS_NewSVGContainerFrame), SIMPLE_SVG_CREATE(generic, NS_NewSVGGenericContainerFrame), { &nsGkAtoms::foreignObject, - FULL_CTOR_FCDATA(FCDATA_DISALLOW_OUT_OF_FLOW, - &nsCSSFrameConstructor::ConstructSVGForeignObjectFrame) }, + FCDATA_WITH_WRAPPING_BLOCK(FCDATA_DISALLOW_OUT_OF_FLOW, + NS_NewSVGForeignObjectFrame, + nsCSSAnonBoxes::mozSVGForeignContent) }, SIMPLE_SVG_CREATE(a, NS_NewSVGAFrame), SIMPLE_SVG_CREATE(altGlyph, NS_NewSVGTSpanFrame), SIMPLE_SVG_CREATE(text, NS_NewSVGTextFrame), @@ -4915,59 +4856,6 @@ nsCSSFrameConstructor::FindSVGData(Element* aElement, return data; } -nsresult -nsCSSFrameConstructor::ConstructSVGForeignObjectFrame(nsFrameConstructorState& aState, - FrameConstructionItem& aItem, - nsIFrame* aParentFrame, - const nsStyleDisplay* aStyleDisplay, - nsFrameItems& aFrameItems, - nsIFrame** aNewFrame) -{ - nsIContent* const content = aItem.mContent; - nsStyleContext* const styleContext = aItem.mStyleContext; - - nsIFrame* newFrame = NS_NewSVGForeignObjectFrame(mPresShell, styleContext); - if (NS_UNLIKELY(!newFrame)) { - return NS_ERROR_OUT_OF_MEMORY; - } - - // We don't allow this frame to be out of flow - InitAndRestoreFrame(aState, content, aParentFrame, nsnull, newFrame); - - nsresult rv = aState.AddChild(newFrame, aFrameItems, content, styleContext, - aParentFrame, false, false); - if (NS_FAILED(rv)) { - return rv; - } - - nsRefPtr innerPseudoStyle; - innerPseudoStyle = mPresShell->StyleSet()-> - ResolveAnonymousBoxStyle(nsCSSAnonBoxes::mozSVGForeignContent, styleContext); - - nsIFrame* blockFrame = NS_NewBlockFormattingContext(mPresShell, - innerPseudoStyle); - if (NS_UNLIKELY(!blockFrame)) { - newFrame->Destroy(); - return NS_ERROR_OUT_OF_MEMORY; - } - - nsFrameItems childItems; - // We should be relatively positioned so that we end up being the - // absolute containing block. - const nsStyleDisplay* disp = innerPseudoStyle->GetStyleDisplay(); - NS_ASSERTION(disp->IsPositioned(), "How did this get to not be positioned?"); - rv = ConstructBlock(aState, innerPseudoStyle->GetStyleDisplay(), content, - newFrame, newFrame, innerPseudoStyle, - &blockFrame, childItems, true, - aItem.mPendingBinding); - - newFrame->SetInitialChildList(kPrincipalList, childItems); - - *aNewFrame = newFrame; - - return rv; -} - void nsCSSFrameConstructor::AddPageBreakItem(nsIContent* aContent, nsStyleContext* aMainStyleContext, @@ -9521,13 +9409,18 @@ nsCSSFrameConstructor::ProcessChildren(nsFrameConstructorState& aState, const bool aCanHaveGeneratedContent, nsFrameItems& aFrameItems, const bool aAllowBlockStyles, - PendingBinding* aPendingBinding) + PendingBinding* aPendingBinding, + nsIFrame* aPossiblyLeafFrame) { NS_PRECONDITION(aFrame, "Must have parent frame here"); NS_PRECONDITION(aFrame->GetContentInsertionFrame() == aFrame, "Parent frame in ProcessChildren should be its own " "content insertion frame"); + if (!aPossiblyLeafFrame) { + aPossiblyLeafFrame = aFrame; + } + // XXXbz ideally, this would do all the pushing of various // containing blocks as needed, so callers don't have to do it... @@ -9563,7 +9456,7 @@ nsCSSFrameConstructor::ProcessChildren(nsFrameConstructorState& aState, // non-anonymous children are processed to ensure that popups are never // constructed before the popupset. nsAutoTArray anonymousItems; - GetAnonymousContent(aContent, aFrame, anonymousItems); + GetAnonymousContent(aContent, aPossiblyLeafFrame, anonymousItems); for (PRUint32 i = 0; i < anonymousItems.Length(); ++i) { nsIContent* content = anonymousItems[i].mContent; #ifdef DEBUG @@ -9600,7 +9493,7 @@ nsCSSFrameConstructor::ProcessChildren(nsFrameConstructorState& aState, itemsToConstruct); } - if (!aFrame->IsLeaf()) { + if (!aPossiblyLeafFrame->IsLeaf()) { // :before/:after content should have the same style context parent // as normal kids. // Note that we don't use this style context for looking up things like diff --git a/layout/base/nsCSSFrameConstructor.h b/layout/base/nsCSSFrameConstructor.h index faf0c01ae1c8..8ed7849bce42 100644 --- a/layout/base/nsCSSFrameConstructor.h +++ b/layout/base/nsCSSFrameConstructor.h @@ -727,6 +727,11 @@ private: would have been scrollable but has been forced to be non-scrollable due to being in a paginated context. */ #define FCDATA_FORCED_NON_SCROLLABLE_BLOCK 0x20000 + /* If FCDATA_CREATE_BLOCK_WRAPPER_FOR_ALL_KIDS is set, then create a + block formatting context wrapper around the kids of this frame + using the FrameConstructionData's mPseudoAtom for its anonymous + box type. */ +#define FCDATA_CREATE_BLOCK_WRAPPER_FOR_ALL_KIDS 0x40000 /* Structure representing information about how a frame should be constructed. */ @@ -745,6 +750,9 @@ private: FrameConstructionDataGetter mDataGetter; } mFunc; FrameFullConstructor mFullConstructor; + // For cases when FCDATA_CREATE_BLOCK_WRAPPER_FOR_ALL_KIDS is set, the + // anonymous box type to use for that wrapper. + nsICSSAnonBoxPseudo * const * const mAnonBoxPseudo; }; /* Structure representing a mapping of an atom to a FrameConstructionData. @@ -1130,15 +1138,6 @@ protected: nsIFrame** aPlaceholderFrame); private: - // ConstructButtonFrame puts the new frame in aFrameItems and - // handles the kids of the button. - nsresult ConstructButtonFrame(nsFrameConstructorState& aState, - FrameConstructionItem& aItem, - nsIFrame* aParentFrame, - const nsStyleDisplay* aStyleDisplay, - nsFrameItems& aFrameItems, - nsIFrame** aNewFrame); - // ConstructSelectFrame puts the new frame in aFrameItems and // handles the kids of the select. nsresult ConstructSelectFrame(nsFrameConstructorState& aState, @@ -1328,13 +1327,6 @@ private: nsIFrame* aParentFrame, nsStyleContext* aStyleContext); - nsresult ConstructSVGForeignObjectFrame(nsFrameConstructorState& aState, - FrameConstructionItem& aItem, - nsIFrame* aParentFrame, - const nsStyleDisplay* aStyleDisplay, - nsFrameItems& aFrameItems, - nsIFrame** aNewFrame); - /* Not static because it does PropagateScrollToViewport. If this changes, make this static */ const FrameConstructionData* @@ -1388,6 +1380,9 @@ private: * styles on the parent. * @param aPendingBinding Make sure to push this into aState before doing any * child item construction. + * @param aPossiblyLeafFrame if non-null, this should be used for the isLeaf + * test and the anonymous content creation. If null, aFrame will be + * used. */ nsresult ProcessChildren(nsFrameConstructorState& aState, nsIContent* aContent, @@ -1396,7 +1391,8 @@ private: const bool aCanHaveGeneratedContent, nsFrameItems& aFrameItems, const bool aAllowBlockStyles, - PendingBinding* aPendingBinding); + PendingBinding* aPendingBinding, + nsIFrame* aPossiblyLeafFrame = nsnull); nsIFrame* GetFrameFor(nsIContent* aContent); diff --git a/layout/reftests/forms/button-first-letter-1-noref.html b/layout/reftests/forms/button-first-letter-1-noref.html new file mode 100644 index 000000000000..d59db575fa6f --- /dev/null +++ b/layout/reftests/forms/button-first-letter-1-noref.html @@ -0,0 +1,5 @@ + + + diff --git a/layout/reftests/forms/button-first-letter-1-ref.html b/layout/reftests/forms/button-first-letter-1-ref.html new file mode 100644 index 000000000000..34ffa64f22ab --- /dev/null +++ b/layout/reftests/forms/button-first-letter-1-ref.html @@ -0,0 +1,7 @@ + + + diff --git a/layout/reftests/forms/button-first-letter-1.html b/layout/reftests/forms/button-first-letter-1.html new file mode 100644 index 000000000000..3dbf1667fa8a --- /dev/null +++ b/layout/reftests/forms/button-first-letter-1.html @@ -0,0 +1,6 @@ + + + diff --git a/layout/reftests/forms/reftest.list b/layout/reftests/forms/reftest.list index 11ded8625746..58efaa7679f9 100644 --- a/layout/reftests/forms/reftest.list +++ b/layout/reftests/forms/reftest.list @@ -61,6 +61,9 @@ fails-if(Android) != textarea-rtl.html textarea-no-resize.html == select-boguskids.html select-boguskids-ref.html == select-dynamic-boguskids.html select-boguskids-ref.html +asserts(2) == button-first-letter-1.html button-first-letter-1-ref.html +asserts(1) != button-first-letter-1.html button-first-letter-1-noref.html + # placeholder include placeholder/reftest.list