From 0e1bf45a0854ea8a1e55dede20c1f8e5819378e9 Mon Sep 17 00:00:00 2001 From: Xidorn Quan Date: Fri, 24 Aug 2018 10:44:04 +0000 Subject: [PATCH] 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 | 2 +- layout/generic/nsLineLayout.cpp | 37 +++++++++++++++++-- layout/generic/nsLineLayout.h | 12 +++--- .../list-item/bullet-justify-1-ref.html | 11 ++++++ .../reftests/list-item/bullet-justify-1.html | 12 ++++++ layout/reftests/list-item/reftest.list | 1 + 6 files changed, 64 insertions(+), 11 deletions(-) create mode 100644 layout/reftests/list-item/bullet-justify-1-ref.html create mode 100644 layout/reftests/list-item/bullet-justify-1.html diff --git a/layout/generic/nsBlockFrame.cpp b/layout/generic/nsBlockFrame.cpp index ecae7b9f0b07..eee54cbc8563 100644 --- a/layout/generic/nsBlockFrame.cpp +++ b/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"); diff --git a/layout/generic/nsLineLayout.cpp b/layout/generic/nsLineLayout.cpp index 1245ab38c916..583847b53ae6 100644 --- a/layout/generic/nsLineLayout.cpp +++ b/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); diff --git a/layout/generic/nsLineLayout.h b/layout/generic/nsLineLayout.h index 38eab887c255..eeff4cc4cd67 100644 --- a/layout/generic/nsLineLayout.h +++ b/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; } diff --git a/layout/reftests/list-item/bullet-justify-1-ref.html b/layout/reftests/list-item/bullet-justify-1-ref.html new file mode 100644 index 000000000000..7512879a0d8b --- /dev/null +++ b/layout/reftests/list-item/bullet-justify-1-ref.html @@ -0,0 +1,11 @@ + + +
  • x x
    • diff --git a/layout/reftests/list-item/bullet-justify-1.html b/layout/reftests/list-item/bullet-justify-1.html new file mode 100644 index 000000000000..22cf2773b29a --- /dev/null +++ b/layout/reftests/list-item/bullet-justify-1.html @@ -0,0 +1,12 @@ + + +
  • x x
    • diff --git a/layout/reftests/list-item/reftest.list b/layout/reftests/list-item/reftest.list index 717579033552..2de55bc641bc 100644 --- a/layout/reftests/list-item/reftest.list +++ b/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