From 2ee74123ac865545912aa82f42b868215daa38c6 Mon Sep 17 00:00:00 2001 From: "L. David Baron" Date: Fri, 11 Sep 2009 06:46:36 -0400 Subject: [PATCH] Make empty bullets (i.e., those for list-style-type:none) not contribute to layout. (Bug 512631) r=roc --- layout/generic/nsBlockFrame.cpp | 68 +++++++++++++++++--------- layout/generic/nsBlockFrame.h | 5 ++ layout/generic/nsLineBox.h | 2 +- layout/generic/nsLineLayout.cpp | 10 +++- layout/reftests/bugs/512631-1-ref.html | 25 ++++++++++ layout/reftests/bugs/512631-1.html | 25 ++++++++++ layout/reftests/bugs/reftest.list | 3 +- 7 files changed, 111 insertions(+), 27 deletions(-) create mode 100644 layout/reftests/bugs/512631-1-ref.html create mode 100644 layout/reftests/bugs/512631-1.html diff --git a/layout/generic/nsBlockFrame.cpp b/layout/generic/nsBlockFrame.cpp index 91e132ea9682..69ee5d8a6171 100644 --- a/layout/generic/nsBlockFrame.cpp +++ b/layout/generic/nsBlockFrame.cpp @@ -1036,8 +1036,10 @@ nsBlockFrame::Reflow(nsPresContext* aPresContext, nscoord lineTop = havePosition ? position.mTop : aReflowState.mComputedBorderPadding.top; ReflowBullet(state, metrics, lineTop); + NS_ASSERTION(!BulletIsEmpty() || metrics.height == 0, + "empty bullet took up space"); - if (havePosition) { + if (havePosition && !BulletIsEmpty()) { // We have some lines to align the bullet with. // Doing the alignment using the baseline will also cater for @@ -2205,30 +2207,34 @@ nsBlockFrame::ReflowDirtyLines(nsBlockReflowState& aState) nsHTMLReflowMetrics metrics; ReflowBullet(aState, metrics, aState.mReflowState.mComputedBorderPadding.top); + NS_ASSERTION(!BulletIsEmpty() || metrics.height == 0, + "empty bullet took up space"); - // There are no lines so we have to fake up some y motion so that - // we end up with *some* height. + if (!BulletIsEmpty()) { + // There are no lines so we have to fake up some y motion so that + // we end up with *some* height. - if (metrics.ascent == nsHTMLReflowMetrics::ASK_FOR_BASELINE && - !nsLayoutUtils::GetFirstLineBaseline(mBullet, &metrics.ascent)) { - metrics.ascent = metrics.height; - } + if (metrics.ascent == nsHTMLReflowMetrics::ASK_FOR_BASELINE && + !nsLayoutUtils::GetFirstLineBaseline(mBullet, &metrics.ascent)) { + metrics.ascent = metrics.height; + } - nsIRenderingContext *rc = aState.mReflowState.rendContext; - nsLayoutUtils::SetFontFromStyle(rc, GetStyleContext()); - nsCOMPtr fm; - rc->GetFontMetrics(*getter_AddRefs(fm)); + nsIRenderingContext *rc = aState.mReflowState.rendContext; + nsLayoutUtils::SetFontFromStyle(rc, GetStyleContext()); + nsCOMPtr fm; + rc->GetFontMetrics(*getter_AddRefs(fm)); - nscoord minAscent = - nsLayoutUtils::GetCenteredFontBaseline(fm, aState.mMinLineHeight); - nscoord minDescent = aState.mMinLineHeight - minAscent; + nscoord minAscent = + nsLayoutUtils::GetCenteredFontBaseline(fm, aState.mMinLineHeight); + nscoord minDescent = aState.mMinLineHeight - minAscent; - aState.mY += PR_MAX(minAscent, metrics.ascent) + - PR_MAX(minDescent, metrics.height - metrics.ascent); + aState.mY += PR_MAX(minAscent, metrics.ascent) + + PR_MAX(minDescent, metrics.height - metrics.ascent); - nscoord offset = minAscent - metrics.ascent; - if (offset > 0) { - mBullet->SetRect(mBullet->GetRect() + nsPoint(0, offset)); + nscoord offset = minAscent - metrics.ascent; + if (offset > 0) { + mBullet->SetRect(mBullet->GetRect() + nsPoint(0, offset)); + } } } @@ -2736,7 +2742,7 @@ nsBlockFrame::IsSelfEmpty() return PR_FALSE; } - if (HaveOutsideBullet()) { + if (HaveOutsideBullet() && !BulletIsEmpty()) { return PR_FALSE; } @@ -4053,6 +4059,8 @@ nsBlockFrame::PlaceLine(nsBlockReflowState& aState, aLine == mLines.begin().next()))) { nsHTMLReflowMetrics metrics; ReflowBullet(aState, metrics, aState.mY); + NS_ASSERTION(!BulletIsEmpty() || metrics.height == 0, + "empty bullet took up space"); aLineLayout.AddBulletFrame(mBullet, metrics); addedBullet = PR_TRUE; } @@ -6155,7 +6163,8 @@ NS_IMETHODIMP nsBlockFrame::GetAccessible(nsIAccessible** aAccessible) } #endif -void nsBlockFrame::ClearLineCursor() { +void nsBlockFrame::ClearLineCursor() +{ if (!(GetStateBits() & NS_BLOCK_HAS_LINE_CURSOR)) { return; } @@ -6164,7 +6173,8 @@ void nsBlockFrame::ClearLineCursor() { RemoveStateBits(NS_BLOCK_HAS_LINE_CURSOR); } -void nsBlockFrame::SetupLineCursor() { +void nsBlockFrame::SetupLineCursor() +{ if (GetStateBits() & NS_BLOCK_HAS_LINE_CURSOR || mLines.empty()) { return; @@ -6175,7 +6185,8 @@ void nsBlockFrame::SetupLineCursor() { AddStateBits(NS_BLOCK_HAS_LINE_CURSOR); } -nsLineBox* nsBlockFrame::GetFirstLineContaining(nscoord y) { +nsLineBox* nsBlockFrame::GetFirstLineContaining(nscoord y) +{ if (!(GetStateBits() & NS_BLOCK_HAS_LINE_CURSOR)) { return nsnull; } @@ -6359,6 +6370,17 @@ nsBlockFrame::SetInitialChildList(nsIAtom* aListName, return NS_OK; } +PRBool +nsBlockFrame::BulletIsEmpty() const +{ + NS_ASSERTION(GetStyleDisplay()->mDisplay == NS_STYLE_DISPLAY_LIST_ITEM && + HaveOutsideBullet(), + "should only care when we have an outside bullet"); + const nsStyleList* list = GetStyleList(); + return list->mListStyleType == NS_STYLE_LIST_STYLE_NONE && + !list->mListStyleImage; +} + // static PRBool nsBlockFrame::FrameStartsCounterScope(nsIFrame* aFrame) diff --git a/layout/generic/nsBlockFrame.h b/layout/generic/nsBlockFrame.h index fe4bffc0ffac..e6bdd29b6407 100644 --- a/layout/generic/nsBlockFrame.h +++ b/layout/generic/nsBlockFrame.h @@ -234,6 +234,11 @@ public: virtual PRBool CachedIsEmpty(); virtual PRBool IsSelfEmpty(); + // Given that we have a bullet, does it actually draw something, i.e., + // do we have either a 'list-style-type' or 'list-style-image' that is + // not 'none'? + PRBool BulletIsEmpty() const; + virtual void MarkIntrinsicWidthsDirty(); virtual nscoord GetMinWidth(nsIRenderingContext *aRenderingContext); virtual nscoord GetPrefWidth(nsIRenderingContext *aRenderingContext); diff --git a/layout/generic/nsLineBox.h b/layout/generic/nsLineBox.h index f40ff1f44b00..cce82d04074b 100644 --- a/layout/generic/nsLineBox.h +++ b/layout/generic/nsLineBox.h @@ -499,7 +499,7 @@ public: PRUint32 mEmptyCacheValid: 1; PRUint32 mEmptyCacheState: 1; // mHasBullet indicates that this is an inline line whose block's - // bullet is adjacent to this line. + // bullet is adjacent to this line and non-empty. PRUint32 mHasBullet : 1; PRUint32 mBreakType : 4; diff --git a/layout/generic/nsLineLayout.cpp b/layout/generic/nsLineLayout.cpp index d8f86ccde2f0..2f08136f483f 100644 --- a/layout/generic/nsLineLayout.cpp +++ b/layout/generic/nsLineLayout.cpp @@ -1332,8 +1332,14 @@ nsLineLayout::AddBulletFrame(nsIFrame* aFrame, NS_ASSERTION(mCurrentSpan == mRootSpan, "bad linelayout user"); NS_ASSERTION(GetFlag(LL_GOTLINEBOX), "must have line box"); - SetFlag(LL_HASBULLET, PR_TRUE); - mLineBox->SetHasBullet(); + + nsIFrame *blockFrame = mBlockReflowState->frame; + NS_ASSERTION(blockFrame->IsFrameOfType(nsIFrame::eBlockFrame), + "must be for block"); + if (!static_cast(blockFrame)->BulletIsEmpty()) { + SetFlag(LL_HASBULLET, PR_TRUE); + mLineBox->SetHasBullet(); + } PerFrameData* pfd; nsresult rv = NewPerFrameData(&pfd); diff --git a/layout/reftests/bugs/512631-1-ref.html b/layout/reftests/bugs/512631-1-ref.html new file mode 100644 index 000000000000..17cf98e15e0c --- /dev/null +++ b/layout/reftests/bugs/512631-1-ref.html @@ -0,0 +1,25 @@ + + + + + +test + + + + + diff --git a/layout/reftests/bugs/512631-1.html b/layout/reftests/bugs/512631-1.html new file mode 100644 index 000000000000..d35b019d0873 --- /dev/null +++ b/layout/reftests/bugs/512631-1.html @@ -0,0 +1,25 @@ + + + + + +test + + + + + diff --git a/layout/reftests/bugs/reftest.list b/layout/reftests/bugs/reftest.list index 1cb374ada2b9..7c323eaee0e0 100644 --- a/layout/reftests/bugs/reftest.list +++ b/layout/reftests/bugs/reftest.list @@ -1311,8 +1311,9 @@ fails-if(MOZ_WIDGET_TOOLKIT!="cocoa") == 488692-1.html 488692-1-ref.html # needs == 507487-2.xhtml 507487-2-ref.xhtml == 508919-1.xhtml 508919-1-ref.xhtml == 509155-1.xhtml 509155-1-ref.xhtml +== 512410.html 512410-ref.html +== 512631-1.html 512631-1-ref.html == 513153-1a.html 513153-1-ref.html == 513153-1b.html 513153-1-ref.html == 513153-2a.html 513153-2-ref.html == 513153-2b.html 513153-2-ref.html -== 512410.html 512410-ref.html