From 2fa4de5805bbeef521db32095e61e459a75398c8 Mon Sep 17 00:00:00 2001 From: "dholbert%cs.stanford.edu" Date: Thu, 26 Feb 2009 20:12:49 +0000 Subject: [PATCH] (Patch #1 to fix Bug 431260) Patch for Bug 431341 - Include floating first-letter text when we build textruns for a paragraph, because we want nsLineBreaker to see the text for capitalization analysis. Make sure that textrun construction for floating first-letter text uses the block as its scope. And make sure we reconstruct textruns after determining the first-letter length, so that ligatures are broken as necessary. Patch by Robert O'Callahan r=smontagu git-svn-id: svn://10.0.0.236/trunk@256331 18797224-902f-48f8-a5cc-f745e15eee43 --- mozilla/layout/generic/nsLineLayout.h | 2 +- mozilla/layout/generic/nsTextFrameThebes.cpp | 173 ++++++++++++------ .../layout/reftests/bugs/431341-1-ref.html | 12 ++ mozilla/layout/reftests/bugs/431341-1.html | 13 ++ .../layout/reftests/bugs/431341-2-ref.html | 12 ++ mozilla/layout/reftests/bugs/431341-2.html | 12 ++ mozilla/layout/reftests/bugs/reftest.list | 2 + 7 files changed, 165 insertions(+), 61 deletions(-) create mode 100644 mozilla/layout/reftests/bugs/431341-1-ref.html create mode 100644 mozilla/layout/reftests/bugs/431341-1.html create mode 100644 mozilla/layout/reftests/bugs/431341-2-ref.html create mode 100644 mozilla/layout/reftests/bugs/431341-2.html diff --git a/mozilla/layout/generic/nsLineLayout.h b/mozilla/layout/generic/nsLineLayout.h index 059dd491772..ae3c470faf0 100644 --- a/mozilla/layout/generic/nsLineLayout.h +++ b/mozilla/layout/generic/nsLineLayout.h @@ -319,7 +319,7 @@ public: /** * This can't be null. It usually returns a block frame but may return * some other kind of frame when inline frames are reflowed in a non-block - * context (e.g. MathML). + * context (e.g. MathML or floating first-letter). */ nsIFrame* GetLineContainerFrame() const { return mBlockReflowState->frame; } const nsLineList::iterator* GetLine() const { diff --git a/mozilla/layout/generic/nsTextFrameThebes.cpp b/mozilla/layout/generic/nsTextFrameThebes.cpp index 9b08a5d2242..f7a2f36a1aa 100644 --- a/mozilla/layout/generic/nsTextFrameThebes.cpp +++ b/mozilla/layout/generic/nsTextFrameThebes.cpp @@ -569,7 +569,8 @@ public: mContext(aContext), mLineContainer(aLineContainer), mBidiEnabled(aPresContext->BidiEnabled()), - mTrimNextRunLeadingWhitespace(PR_FALSE), mSkipIncompleteTextRuns(PR_FALSE) { + mTrimNextRunLeadingWhitespace(PR_FALSE), + mSkipIncompleteTextRuns(PR_FALSE) { ResetRunInfo(); } @@ -717,7 +718,7 @@ private: static nsIFrame* FindLineContainer(nsIFrame* aFrame) { - while (aFrame && aFrame->IsFrameOfType(nsIFrame::eLineParticipant)) { + while (aFrame && aFrame->CanContinueTextRun()) { aFrame = aFrame->GetParent(); } return aFrame; @@ -745,20 +746,59 @@ TextContainsLineBreakerWhiteSpace(const void* aText, PRUint32 aLength, } } -static PRBool -CanTextRunCrossFrameBoundary(nsIFrame* aFrame) +struct FrameTextTraversal { + nsIFrame* mFrameToDescendInto; + PRPackedBool mDescendIntoFrameSiblings; + PRPackedBool mLineBreakerCanCrossFrameBoundary; + PRPackedBool mTextRunCanCrossFrameBoundary; +}; + +static FrameTextTraversal +CanTextCrossFrameBoundary(nsIFrame* aFrame, nsIAtom* aType) { - // placeholders are "invisible", so a text run should be able to span - // across one. The text in the out-of-flow, if any, will not be included - // in this textrun of course. - return aFrame->CanContinueTextRun() || - aFrame->GetType() == nsGkAtoms::placeholderFrame; + NS_ASSERTION(aType == aFrame->GetType(), "Wrong type"); + + FrameTextTraversal result; + + PRBool continuesTextRun = aFrame->CanContinueTextRun(); + if (aType == nsGkAtoms::placeholderFrame) { + // placeholders are "invisible", so a text run should be able to span + // across one. But don't descend into the out-of-flow. + result.mLineBreakerCanCrossFrameBoundary = PR_TRUE; + if (continuesTextRun) { + // ... Except for first-letter floats, which are really in-flow + // from the point of view of capitalization etc, so we'd better + // descend into them. But we actually need to break the textrun for + // first-letter floats since things look bad if, say, we try to make a + // ligature across the float boundary. + result.mFrameToDescendInto = + (static_cast(aFrame))->GetOutOfFlowFrame(); + result.mDescendIntoFrameSiblings = PR_FALSE; + result.mTextRunCanCrossFrameBoundary = PR_FALSE; + } else { + result.mFrameToDescendInto = nsnull; + result.mTextRunCanCrossFrameBoundary = PR_TRUE; + } + } else { + if (continuesTextRun) { + result.mFrameToDescendInto = aFrame->GetFirstChild(nsnull); + result.mDescendIntoFrameSiblings = PR_TRUE; + result.mTextRunCanCrossFrameBoundary = PR_TRUE; + result.mLineBreakerCanCrossFrameBoundary = PR_TRUE; + } else { + result.mFrameToDescendInto = nsnull; + result.mTextRunCanCrossFrameBoundary = PR_FALSE; + result.mLineBreakerCanCrossFrameBoundary = PR_FALSE; + } + } + return result; } BuildTextRunsScanner::FindBoundaryResult BuildTextRunsScanner::FindBoundaries(nsIFrame* aFrame, FindBoundaryState* aState) { - nsTextFrame* textFrame = aFrame->GetType() == nsGkAtoms::textFrame + nsIAtom* frameType = aFrame->GetType(); + nsTextFrame* textFrame = frameType == nsGkAtoms::textFrame ? static_cast(aFrame) : nsnull; if (textFrame) { if (aState->mLastTextFrame && @@ -794,28 +834,24 @@ BuildTextRunsScanner::FindBoundaries(nsIFrame* aFrame, FindBoundaryState* aState return FB_CONTINUE; } - PRBool continueTextRun = CanTextRunCrossFrameBoundary(aFrame); - PRBool descendInto = PR_TRUE; - if (!continueTextRun) { - // XXX do we need this? are there frames we need to descend into that aren't - // float-containing-blocks? - descendInto = !aFrame->IsFloatContainingBlock(); + FrameTextTraversal traversal = + CanTextCrossFrameBoundary(aFrame, frameType); + if (!traversal.mTextRunCanCrossFrameBoundary) { aState->mSeenTextRunBoundaryOnThisLine = PR_TRUE; if (aState->mSeenSpaceForLineBreakingOnThisLine) return FB_FOUND_VALID_TEXTRUN_BOUNDARY; } - if (descendInto) { - nsIFrame* child = aFrame->GetFirstChild(nsnull); - while (child) { - FindBoundaryResult result = FindBoundaries(child, aState); - if (result != FB_CONTINUE) - return result; - child = child->GetNextSibling(); - } + for (nsIFrame* f = traversal.mFrameToDescendInto; f; + f = f->GetNextSibling()) { + FindBoundaryResult result = FindBoundaries(f, aState); + if (result != FB_CONTINUE) + return result; + if (!traversal.mDescendIntoFrameSiblings) + break; } - if (!continueTextRun) { + if (!traversal.mTextRunCanCrossFrameBoundary) { aState->mSeenTextRunBoundaryOnThisLine = PR_TRUE; if (aState->mSeenSpaceForLineBreakingOnThisLine) return FB_FOUND_VALID_TEXTRUN_BOUNDARY; @@ -841,10 +877,10 @@ static void BuildTextRuns(gfxContext* aContext, nsTextFrame* aForFrame, nsIFrame* aLineContainer, const nsLineList::iterator* aForFrameLine) { - NS_ASSERTION(aForFrame || aForFrameLine, - "One of aForFrame or aForFrameLine must be set!"); + NS_ASSERTION(aForFrame || (aForFrameLine && aLineContainer), + "One of aForFrame or aForFrameLine+aLineContainer must be set!"); - if (!aLineContainer) { + if (!aLineContainer || !aForFrameLine) { aLineContainer = FindLineContainer(aForFrame); } else { NS_ASSERTION(!aForFrame || aLineContainer == FindLineContainer(aForFrame), "Wrong line container hint"); @@ -1207,34 +1243,35 @@ void BuildTextRunsScanner::ScanFrame(nsIFrame* aFrame) return; } - PRBool continueTextRun = CanTextRunCrossFrameBoundary(aFrame); - PRBool descendInto = PR_TRUE; + FrameTextTraversal traversal = + CanTextCrossFrameBoundary(aFrame, frameType); PRBool isBR = frameType == nsGkAtoms::brFrame; - if (!continueTextRun) { + if (!traversal.mLineBreakerCanCrossFrameBoundary) { // BR frames are special. We do not need or want to record a break opportunity // before a BR frame. FlushFrames(PR_TRUE, isBR); mCommonAncestorWithLastFrame = aFrame; mTrimNextRunLeadingWhitespace = PR_FALSE; - // XXX do we need this? are there frames we need to descend into that aren't - // float-containing-blocks? - descendInto = !aFrame->IsFloatContainingBlock(); mStartOfLine = PR_FALSE; + } else if (!traversal.mTextRunCanCrossFrameBoundary) { + FlushFrames(PR_FALSE, PR_FALSE); } - if (descendInto) { - nsIFrame* f; - for (f = aFrame->GetFirstChild(nsnull); f; f = f->GetNextSibling()) { - ScanFrame(f); - } + for (nsIFrame* f = traversal.mFrameToDescendInto; f; + f = f->GetNextSibling()) { + ScanFrame(f); + if (!traversal.mDescendIntoFrameSiblings) + break; } - if (!continueTextRun) { + if (!traversal.mLineBreakerCanCrossFrameBoundary) { // Really if we're a BR frame this is unnecessary since descendInto will be // false. In fact this whole "if" statement should move into the descendInto. FlushFrames(PR_TRUE, isBR); mCommonAncestorWithLastFrame = aFrame; mTrimNextRunLeadingWhitespace = PR_FALSE; + } else if (!traversal.mTextRunCanCrossFrameBoundary) { + FlushFrames(PR_FALSE, PR_FALSE); } LiftCommonAncestorWithLastFrameToParent(aFrame->GetParent()); @@ -3192,7 +3229,7 @@ nsContinuingTextFrame::Init(nsIContent* aContent, aPrevInFlow->SetNextInFlow(this); nsTextFrame* prev = static_cast(aPrevInFlow); mContentOffset = prev->GetContentOffset() + prev->GetContentLengthHint(); - NS_ASSERTION(mContentOffset < aContent->GetText()->GetLength(), + NS_ASSERTION(mContentOffset < PRInt32(aContent->GetText()->GetLength()), "Creating ContinuingTextFrame, but there is no more content"); if (prev->GetStyleContext() != GetStyleContext()) { // We're taking part of prev's text, and its style may be different @@ -5382,15 +5419,11 @@ nsTextFrame::Reflow(nsPresContext* aPresContext, AddStateBits(TEXT_START_OF_LINE); } - // Layout dependent styles are a problem because we need to reconstruct - // the gfxTextRun based on our layout. - PRBool layoutDependentTextRun = - lineLayout.GetFirstLetterStyleOK() || lineLayout.GetInFirstLine(); - if (layoutDependentTextRun) { - SetLength(maxContentLength); - } - + PRUint32 flowEndInTextRun; + nsIFrame* lineContainer = lineLayout.GetLineContainerFrame(); + gfxContext* ctx = aReflowState.rendContext->ThebesContext(); const nsTextFragment* frag = mContent->GetText(); + // DOM offsets of the text range we need to measure, after trimming // whitespace, restricting to first-letter, and restricting preformatted text // to nearest newline @@ -5413,9 +5446,36 @@ nsTextFrame::Reflow(nsPresContext* aPresContext, } } - PRUint32 flowEndInTextRun; - nsIFrame* lineContainer = lineLayout.GetLineContainerFrame(); - gfxContext* ctx = aReflowState.rendContext->ThebesContext(); + PRBool completedFirstLetter = PR_FALSE; + // Layout dependent styles are a problem because we need to reconstruct + // the gfxTextRun based on our layout. + if (lineLayout.GetFirstLetterStyleOK() || lineLayout.GetInFirstLine()) { + SetLength(maxContentLength); + + if (lineLayout.GetFirstLetterStyleOK()) { + // floating first-letter boundaries are significant in textrun + // construction, so clear the textrun out every time we hit a first-letter + // and have changed our length (which controls the first-letter boundary) + ClearTextRun(); + // Find the length of the first-letter. We need a textrun for this. + gfxSkipCharsIterator iter = + EnsureTextRun(ctx, lineContainer, lineLayout.GetLine(), &flowEndInTextRun); + + if (mTextRun) { + completedFirstLetter = FindFirstLetterRange(frag, mTextRun, offset, iter, &length); + if (length) { + AddStateBits(TEXT_FIRST_LETTER); + } + // Change this frame's length to the first-letter length right now + // so that when we rebuild the textrun it will be built with the + // right first-letter boundary + SetLength(offset + length - GetContentOffset()); + // Ensure that the textrun will be rebuilt + ClearTextRun(); + } + } + } + gfxSkipCharsIterator iter = EnsureTextRun(ctx, lineContainer, lineLayout.GetLine(), &flowEndInTextRun); @@ -5431,7 +5491,7 @@ nsTextFrame::Reflow(nsPresContext* aPresContext, iter = EnsureTextRun(ctx, lineContainer, lineLayout.GetLine(), &flowEndInTextRun); } - + if (!mTextRun) { ClearMetrics(aMetrics); aStatus = NS_FRAME_COMPLETE; @@ -5442,13 +5502,6 @@ nsTextFrame::Reflow(nsPresContext* aPresContext, <= mTextRun->GetLength(), "Text run does not map enough text for our reflow"); - // Restrict to just the first-letter if necessary - PRBool completedFirstLetter = PR_FALSE; - if (lineLayout.GetFirstLetterStyleOK()) { - AddStateBits(TEXT_FIRST_LETTER); - completedFirstLetter = FindFirstLetterRange(frag, mTextRun, offset, iter, &length); - } - ///////////////////////////////////////////////////////////////////// // See how much text should belong to this text frame, and measure it ///////////////////////////////////////////////////////////////////// diff --git a/mozilla/layout/reftests/bugs/431341-1-ref.html b/mozilla/layout/reftests/bugs/431341-1-ref.html new file mode 100644 index 00000000000..b70e7bd5f3c --- /dev/null +++ b/mozilla/layout/reftests/bugs/431341-1-ref.html @@ -0,0 +1,12 @@ + + + + + + +

Hello Kitty +

"Hello Kitty" + + diff --git a/mozilla/layout/reftests/bugs/431341-1.html b/mozilla/layout/reftests/bugs/431341-1.html new file mode 100644 index 00000000000..44b80657e0c --- /dev/null +++ b/mozilla/layout/reftests/bugs/431341-1.html @@ -0,0 +1,13 @@ + + + + + + +

hello kitty +

"hello kitty" + + diff --git a/mozilla/layout/reftests/bugs/431341-2-ref.html b/mozilla/layout/reftests/bugs/431341-2-ref.html new file mode 100644 index 00000000000..5fa3d7e220d --- /dev/null +++ b/mozilla/layout/reftests/bugs/431341-2-ref.html @@ -0,0 +1,12 @@ + + + + + + +

f‌ish fish f‌ish + + diff --git a/mozilla/layout/reftests/bugs/431341-2.html b/mozilla/layout/reftests/bugs/431341-2.html new file mode 100644 index 00000000000..96377592b27 --- /dev/null +++ b/mozilla/layout/reftests/bugs/431341-2.html @@ -0,0 +1,12 @@ + + + + + + +

fish fish f‌ish + + diff --git a/mozilla/layout/reftests/bugs/reftest.list b/mozilla/layout/reftests/bugs/reftest.list index 39feb8b63d7..47677e2a0b6 100644 --- a/mozilla/layout/reftests/bugs/reftest.list +++ b/mozilla/layout/reftests/bugs/reftest.list @@ -827,6 +827,8 @@ fails-if(MOZ_WIDGET_TOOLKIT=="gtk2") == 424074-1-ref2.xul 424074-1-ref3.xul == 430813-1.html 430813-1-ref.html == 430813-2.html 430813-2-ref.html == 430813-3.html 430813-3-ref.html +== 431341-1.html 431341-1-ref.html +== 431341-2.html 431341-2-ref.html == 433640-1.html 433640-1-ref.html == 433700.html 433700-ref.html == 436356-1.html 436356-1-ref.html