Make empty bullets (i.e., those for list-style-type:none) not contribute to layout. (Bug 512631) r=roc

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<nsIFontMetrics> fm;
-    rc->GetFontMetrics(*getter_AddRefs(fm));
+      nsIRenderingContext *rc = aState.mReflowState.rendContext;
+      nsLayoutUtils::SetFontFromStyle(rc, GetStyleContext());
+      nsCOMPtr<nsIFontMetrics> 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)

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);

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;
 

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<nsBlockFrame*>(blockFrame)->BulletIsEmpty()) {
+    SetFlag(LL_HASBULLET, PR_TRUE);
+    mLineBox->SetHasBullet();
+  }
 
   PerFrameData* pfd;
   nsresult rv = NewPerFrameData(&pfd);

layout/reftests/bugs/512631-1-ref.html

@@ -0,0 +1,25 @@
+<!DOCTYPE HTML>
+<html lang="en">
+<head>
+<style type="text/css">
+html {
+	color: rgb(10,10,10);
+	background: rgb(150,150,150);
+	
+	font-family:sans-serif;
+	}
+ul, li { display: block; }
+a {display:block; float:left; width:100px; height:50px; text-align:center; background: #ccc;}
+
+</style>
+
+<title>test</title>
+</head>
+<body>
+<ul>
+<li><a href="/">abc</a></li>
+<li><a href="/">abc</a></li>
+<li><a href="/">abc</a></li>
+</ul>
+</body>
+</html>

layout/reftests/bugs/512631-1.html

@@ -0,0 +1,25 @@
+<!DOCTYPE HTML>
+<html lang="en">
+<head>
+<style type="text/css">
+html {
+	color: rgb(10,10,10);
+	background: rgb(150,150,150);
+	
+	font-family:sans-serif;
+	}
+ul {list-style:none;}
+a {display:block; float:left; width:100px; height:50px; text-align:center; background: #ccc;}
+
+</style>
+
+<title>test</title>
+</head>
+<body>
+<ul>
+<li><a href="/">abc</a></li>
+<li><a href="/">abc</a></li>
+<li><a href="/">abc</a></li>
+</ul>
+</body>
+</html>

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