Bug 1484521 - Prepend bullet frame in line layout. r=jfkthame

Bug 1478178 regressed this case because bullet frame is the last frame
added to line layout, rather than the first, so when we try to apply
justification, we end up giving it the accumulated offset of the whole
line.

Bullet frame has to be added after other frames in the line have been
placed, because its presence may depend on whether the line is empty.
However, bullet frame is logically the first frame in a line and
appending it to the end is somewhat counter-intuitive.

Thus, this patch tries to fix the issue via prepending bullet frame in
line layout, so that the order of frames there can be more reliable.

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

--HG--
extra : moz-landing-system : lando

layout/generic/nsBlockFrame.cpp

@@ -4612,7 +4612,7 @@ nsBlockFrame::PlaceLine(BlockReflowInput& aState,
         0 == mLines.front()->BSize() &&
         aLine == mLines.begin().next()))) {
     ReflowOutput metrics(aState.mReflowInput);
-    nsIFrame* bullet = GetOutsideBullet();
+    nsBulletFrame* bullet = GetOutsideBullet();
     ReflowBullet(bullet, aState, metrics, aState.mBCoord);
     NS_ASSERTION(!BulletIsEmpty() || metrics.BSize(wm) == 0,
                  "empty bullet took up space");

layout/generic/nsLineLayout.cpp

@@ -13,6 +13,7 @@
 #include "LayoutLogging.h"
 #include "SVGTextFrame.h"
 #include "nsBlockFrame.h"
+#include "nsBulletFrame.h"
 #include "nsFontMetrics.h"
 #include "nsStyleConsts.h"
 #include "nsContainerFrame.h"
@@ -1448,7 +1449,7 @@ nsLineLayout::PlaceFrame(PerFrameData* pfd, ReflowOutput& aMetrics)
 }
 
 void
-nsLineLayout::AddBulletFrame(nsIFrame* aFrame,
+nsLineLayout::AddBulletFrame(nsBulletFrame* aFrame,
                              const ReflowOutput& aMetrics)
 {
   NS_ASSERTION(mCurrentSpan == mRootSpan, "bad linelayout user");
@@ -1464,7 +1465,14 @@ nsLineLayout::AddBulletFrame(nsIFrame* aFrame,
 
   WritingMode lineWM = mRootSpan->mWritingMode;
   PerFrameData* pfd = NewPerFrameData(aFrame);
-  mRootSpan->AppendFrame(pfd);
+  PerSpanData* psd = mRootSpan;
+
+  MOZ_ASSERT(psd->mFirstFrame, "adding bullet to an empty line?");
+  // Prepend the bullet frame to the line.
+  psd->mFirstFrame->mPrev = pfd;
+  pfd->mNext = psd->mFirstFrame;
+  psd->mFirstFrame = pfd;
+
   pfd->mIsBullet = true;
   if (aMetrics.BlockStartAscent() == ReflowOutput::ASK_FOR_BASELINE) {
     pfd->mAscent = aFrame->GetLogicalBaseline(lineWM);
@@ -1477,6 +1485,21 @@ nsLineLayout::AddBulletFrame(nsIFrame* aFrame,
   pfd->mOverflowAreas = aMetrics.mOverflowAreas;
 }
 
+void
+nsLineLayout::RemoveBulletFrame(nsBulletFrame* aFrame)
+{
+  PerSpanData* psd = mCurrentSpan;
+  MOZ_ASSERT(psd == mRootSpan, "bullet on non-root span?");
+  MOZ_ASSERT(psd->mFirstFrame->mFrame == aFrame,
+             "bullet is not the first frame?");
+  PerFrameData* pfd = psd->mFirstFrame;
+  MOZ_ASSERT(pfd != psd->mLastFrame,
+             "bullet is the only frame?");
+  pfd->mNext->mPrev = nullptr;
+  psd->mFirstFrame = pfd->mNext;
+  FreeFrame(pfd);
+}
+
 #ifdef DEBUG
 void
 nsLineLayout::DumpPerSpanData(PerSpanData* psd, int32_t aIndent)
@@ -3286,7 +3309,15 @@ nsLineLayout::TextAlignLine(nsLineBox* aLine,
 
   if (mPresContext->BidiEnabled() &&
       (!mPresContext->IsVisualMode() || !lineWM.IsBidiLTR())) {
-    nsBidiPresUtils::ReorderFrames(psd->mFirstFrame->mFrame,
+    PerFrameData* startFrame = psd->mFirstFrame;
+    MOZ_ASSERT(startFrame, "empty line?");
+    if (startFrame->mIsBullet) {
+      // Bullet shouldn't participate in bidi reordering.
+      startFrame = startFrame->mNext;
+      MOZ_ASSERT(startFrame, "no frame after bullet?");
+      MOZ_ASSERT(!startFrame->mIsBullet, "multiple bullets?");
+    }
+    nsBidiPresUtils::ReorderFrames(startFrame->mFrame,
                                    aLine->GetChildCount(),
                                    lineWM, mContainerSize,
                                    psd->mIStart + mTextIndent + dx);

layout/generic/nsLineLayout.h

@@ -16,6 +16,7 @@
 #include "BlockReflowInput.h"
 #include "nsLineBox.h"
 
+class nsBulletFrame;
 class nsFloatManager;
 struct nsStyleText;
 
@@ -101,11 +102,9 @@ public:
                    ReflowOutput* aMetrics,
                    bool& aPushedFrame);
 
-  void AddBulletFrame(nsIFrame* aFrame, const ReflowOutput& aMetrics);
+  void AddBulletFrame(nsBulletFrame* aFrame, const ReflowOutput& aMetrics);
 
-  void RemoveBulletFrame(nsIFrame* aFrame) {
-    PushFrame(aFrame);
-  }
+  void RemoveBulletFrame(nsBulletFrame* aFrame);
 
   /**
    * Place frames in the block direction (CSS property vertical-align)
@@ -547,10 +546,9 @@ protected:
     nscoord* mBaseline;
 
     void AppendFrame(PerFrameData* pfd) {
-      if (nullptr == mLastFrame) {
+      if (!mLastFrame) {
         mFirstFrame = pfd;
-      }
-      else {
+      } else {
         mLastFrame->mNext = pfd;
         pfd->mPrev = mLastFrame;
       }

layout/reftests/list-item/bullet-justify-1-ref.html

@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<style>
+  li {
+    margin-left: 100px;
+    font-size: 100px;
+  }
+  span {
+    color: transparent;
+  }
+</style>
+<li><span>x x</span></li>

layout/reftests/list-item/bullet-justify-1.html

@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<style>
+  li {
+    margin-left: 100px;
+    font-size: 100px;
+    text-align-last: justify;
+  }
+  span {
+    color: transparent;
+  }
+</style>
+<li><span>x x</span></li>

layout/reftests/list-item/reftest.list

@@ -12,3 +12,4 @@ asserts(1) == ol-reversed-1b.html ol-reversed-1-ref.html # bug 478135
 == bullet-space-2.html bullet-space-2-ref.html
 == bullet-intrinsic-isize-1.html bullet-intrinsic-isize-1-ref.html
 == bullet-intrinsic-isize-2.html bullet-intrinsic-isize-2-ref.html
+== bullet-justify-1.html bullet-justify-1-ref.html