Bug 399531. Rework TrimTrailingWhitespace so that we recompute the overflow area for trimmed textframes and so that soft hyphens in otherwise-empty textframes are activated. r+sr=dbaron
authorroc+@cs.cmu.edu
Sat, 01 Dec 2007 01:24:24 -0800
changeset 8514 b4f2447ed91af4a0ae4840fb63f86268025b5a01
parent 8513 8178839996f1e28d76b9f13c5a1627f8e9550521
child 8515 8b6848a7ead10073667047714da34c92ee9bc536
push idunknown
push userunknown
push dateunknown
bugs399531
milestone1.9b2pre
Bug 399531. Rework TrimTrailingWhitespace so that we recompute the overflow area for trimmed textframes and so that soft hyphens in otherwise-empty textframes are activated. r+sr=dbaron
layout/generic/nsFrame.cpp
layout/generic/nsFrame.h
layout/generic/nsIFrame.h
layout/generic/nsLineLayout.cpp
layout/generic/nsTextFrame.h
layout/generic/nsTextFrameThebes.cpp
layout/reftests/reftest.list
layout/reftests/text/soft-hyphens-1-ref.html
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -3206,27 +3206,16 @@ nsFrame::Reflow(nsPresContext*          
   aDesiredSize.width = 0;
   aDesiredSize.height = 0;
   aStatus = NS_FRAME_COMPLETE;
   NS_FRAME_SET_TRUNCATION(aStatus, aReflowState, aDesiredSize);
   return NS_OK;
 }
 
 NS_IMETHODIMP
-nsFrame::TrimTrailingWhiteSpace(nsPresContext* aPresContext,
-                                nsIRenderingContext& aRC,
-                                nscoord& aDeltaWidth,
-                                PRBool& aLastCharIsJustifiable)
-{
-  aDeltaWidth = 0;
-  aLastCharIsJustifiable = PR_FALSE;
-  return NS_OK;
-}
-
-NS_IMETHODIMP
 nsFrame::CharacterDataChanged(nsPresContext* aPresContext,
                               nsIContent*     aChild,
                               PRBool          aAppend)
 {
   NS_NOTREACHED("should only be called for text frames");
   return NS_OK;
 }
 
--- a/layout/generic/nsFrame.h
+++ b/layout/generic/nsFrame.h
@@ -324,20 +324,16 @@ public:
   NS_IMETHOD  Reflow(nsPresContext*          aPresContext,
                      nsHTMLReflowMetrics&     aDesiredSize,
                      const nsHTMLReflowState& aReflowState,
                      nsReflowStatus&          aStatus);
   NS_IMETHOD  DidReflow(nsPresContext*           aPresContext,
                         const nsHTMLReflowState*  aReflowState,
                         nsDidReflowStatus         aStatus);
   virtual PRBool CanContinueTextRun() const;
-  NS_IMETHOD TrimTrailingWhiteSpace(nsPresContext* aPresContext,
-                                    nsIRenderingContext& aRC,
-                                    nscoord& aDeltaWidth,
-                                    PRBool& aLastCharIsJustifiable);
 
   // Selection Methods
   // XXX Doc me... (in nsIFrame.h puhleeze)
   // XXX If these are selection specific, then the name should imply selection
   // rather than generic event processing, e.g., SelectionHandlePress...
   NS_IMETHOD HandlePress(nsPresContext* aPresContext,
                          nsGUIEvent *    aEvent,
                          nsEventStatus*  aEventStatus);
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -98,21 +98,21 @@ class nsLineList_iterator;
 struct nsPeekOffsetStruct;
 struct nsPoint;
 struct nsRect;
 struct nsSize;
 struct nsMargin;
 
 typedef class nsIFrame nsIBox;
 
-// IID for the nsIFrame interface 
-// 95f75c0a-de85-437a-a195-0304df3f62ce
-#define NS_IFRAME_IID \
-{ 0x95f75c0a, 0xde85, 0x437a, \
-  { 0xa1, 0x95, 0x03, 0x04, 0xdf, 0x3f, 0x62, 0xce } }
+// IID for the nsIFrame interface
+// 04a7dee5-3435-47dc-bd42-a36c0f66a42c
+  #define NS_IFRAME_IID \
+{ 0x04a7dee5, 0x3435, 0x47dc, \
+  { 0xbd, 0x42, 0xa3, 0x6c, 0x0f, 0x66, 0xa4, 0x2c } }
 
 /**
  * Indication of how the frame can be split. This is used when doing runaround
  * of floats, and when pulling up child frames from a next-in-flow.
  *
  * The choices are splittable, not splittable at all, and splittable in
  * a non-rectangular fashion. This last type only applies to block-level
  * elements, and indicates whether splitting can be used when doing runaround.
@@ -1425,23 +1425,16 @@ public:
    *
    * @return 
    *    PR_TRUE if we can continue a "text run" through the frame. A
    *    text run is text that should be treated contiguously for line
    *    and word breaking.
    */
   virtual PRBool CanContinueTextRun() const = 0;
 
-  // Justification helper method that is used to remove trailing
-  // whitespace before justification.
-  NS_IMETHOD TrimTrailingWhiteSpace(nsPresContext* aPresContext,
-                                    nsIRenderingContext& aRC,
-                                    nscoord& aDeltaWidth,
-                                    PRBool& aLastCharIsJustifiable) = 0;
-
   /**
    * Append the rendered text to the passed-in string.
    * The appended text will often not contain all the whitespace from source,
    * depending on whether the CSS rule "white-space: pre" is active for this frame.
    * if aStartOffset + aLength goes past end, or if aLength is not specified
    * then use the text up to the string's end.
    * Call this on the primary frame for a text node.
    * @param aAppendToString   String to append text to, or null if text should not be returned
--- a/layout/generic/nsLineLayout.cpp
+++ b/layout/generic/nsLineLayout.cpp
@@ -2278,70 +2278,75 @@ nsLineLayout::TrimTrailingWhiteSpaceIn(P
     }
     else if (!pfd->GetFlag(PFD_ISTEXTFRAME) &&
              !pfd->GetFlag(PFD_SKIPWHENTRIMMINGWHITESPACE)) {
       // If we hit a frame on the end that's not text and not a placeholder,
       // then there is no trailing whitespace to trim. Stop the search.
       *aDeltaWidth = 0;
       return PR_TRUE;
     }
-    else if (pfd->GetFlag(PFD_ISNONEMPTYTEXTFRAME)) {
-      nscoord deltaWidth = 0;
-      PRBool lastCharIsJustifiable = PR_FALSE;
-      pfd->mFrame->TrimTrailingWhiteSpace(mPresContext,
-                                          *mBlockReflowState->rendContext,
-                                          deltaWidth,
-                                          lastCharIsJustifiable);
+    else if (pfd->GetFlag(PFD_ISTEXTFRAME)) {
+      // Call TrimTrailingWhiteSpace even on empty textframes because they
+      // might have a soft hyphen which should now appear, changing the frame's
+      // width
+      nsTextFrame::TrimOutput trimOutput = static_cast<nsTextFrame*>(pfd->mFrame)->
+          TrimTrailingWhiteSpace(mBlockReflowState->rendContext);
 #ifdef NOISY_TRIM
       nsFrame::ListTag(stdout, (psd == mRootSpan
                                 ? mBlockReflowState->frame
                                 : psd->mFrame->mFrame));
       printf(": trim of ");
       nsFrame::ListTag(stdout, pfd->mFrame);
-      printf(" returned %d\n", deltaWidth);
+      printf(" returned %d\n", trimOutput.mDeltaWidth);
 #endif
-      if (lastCharIsJustifiable && pfd->mJustificationNumSpaces > 0) {
+      if (trimOutput.mLastCharIsJustifiable && pfd->mJustificationNumSpaces > 0) {
         pfd->mJustificationNumSpaces--;
       }
+      
+      if (trimOutput.mChanged) {
+        pfd->SetFlag(PFD_RECOMPUTEOVERFLOW, PR_TRUE);
+      }
 
-      if (deltaWidth) {
-        pfd->mBounds.width -= deltaWidth;
+      if (trimOutput.mDeltaWidth) {
+        pfd->mBounds.width -= trimOutput.mDeltaWidth;
 
         // See if the text frame has already been placed in its parent
         if (psd != mRootSpan) {
           // The frame was already placed during psd's
           // reflow. Update the frames rectangle now.
           pfd->mFrame->SetRect(pfd->mBounds);
         }
 
         // Adjust containing span's right edge
-        psd->mX -= deltaWidth;
+        psd->mX -= trimOutput.mDeltaWidth;
 
         // Slide any frames that follow the text frame over by the
         // right amount. The only thing that can follow the text
         // frame is empty stuff, so we are just making things
         // sensible (keeping the combined area honest).
         while (pfd->mNext) {
           pfd = pfd->mNext;
-          pfd->mBounds.x -= deltaWidth;
+          pfd->mBounds.x -= trimOutput.mDeltaWidth;
           if (psd != mRootSpan) {
             // When the child span is not a direct child of the block
             // we need to update the child spans frame rectangle
             // because it most likely will not be done again. Spans
             // that are direct children of the block will be updated
             // later, however, because the VerticalAlignFrames method
             // will be run after this method.
-            SlideSpanFrameRect(pfd->mFrame, deltaWidth);
+            SlideSpanFrameRect(pfd->mFrame, trimOutput.mDeltaWidth);
           }
         }
       }
 
-      // Pass up to caller so they can shrink their span
-      *aDeltaWidth = deltaWidth;
-      return PR_TRUE;
+      if (pfd->GetFlag(PFD_ISNONEMPTYTEXTFRAME) || trimOutput.mChanged) {
+        // Pass up to caller so they can shrink their span
+        *aDeltaWidth = trimOutput.mDeltaWidth;
+        return PR_TRUE;
+      }
     }
     pfd = pfd->mPrev;
   }
 
   *aDeltaWidth = 0;
   return PR_FALSE;
 }
 
--- a/layout/generic/nsTextFrame.h
+++ b/layout/generic/nsTextFrame.h
@@ -203,20 +203,31 @@ public:
                              nsSize aMargin, nsSize aBorder, nsSize aPadding,
                              PRBool aShrinkWrap);
   virtual nsRect ComputeTightBounds(gfxContext* aContext) const;
   NS_IMETHOD Reflow(nsPresContext* aPresContext,
                     nsHTMLReflowMetrics& aMetrics,
                     const nsHTMLReflowState& aReflowState,
                     nsReflowStatus& aStatus);
   virtual PRBool CanContinueTextRun() const;
-  NS_IMETHOD TrimTrailingWhiteSpace(nsPresContext* aPresContext,
-                                    nsIRenderingContext& aRC,
-                                    nscoord& aDeltaWidth,
-                                    PRBool& aLastCharIsJustifiable);
+  // Method that is called for a text frame that is logically
+  // adjacent to the end of the line (i.e. followed only by empty text frames,
+  // placeholders or inlines containing such).
+  struct TrimOutput {
+    // true if we trimmed some space or changed metrics in some other way.
+    // In this case, we should call RecomputeOverflowRect on this frame.
+    PRPackedBool mChanged;
+    // true if the last character is not justifiable so should be subtracted
+    // from the count of justifiable characters in the frame, since the last
+    // character in a line is not justifiable.
+    PRPackedBool mLastCharIsJustifiable;
+    // an amount to *subtract* from the frame's width (zero if !mChanged)
+    nscoord      mDeltaWidth;
+  };
+  TrimOutput TrimTrailingWhiteSpace(nsIRenderingContext* aRC);
   virtual nsresult GetRenderedText(nsAString* aString = nsnull,
                                    gfxSkipChars* aSkipChars = nsnull,
                                    gfxSkipCharsIterator* aSkipIter = nsnull,
                                    PRUint32 aSkippedStartOffset = 0,
                                    PRUint32 aSkippedMaxLength = PR_UINT32_MAX);
 
   nsRect RecomputeOverflowRect();
 
--- a/layout/generic/nsTextFrameThebes.cpp
+++ b/layout/generic/nsTextFrameThebes.cpp
@@ -2012,16 +2012,21 @@ public:
 
   void GetSpacingInternal(PRUint32 aStart, PRUint32 aLength, Spacing* aSpacing,
                           PRBool aIgnoreTabs);
 
   /**
    * Count the number of justifiable characters in the given DOM range
    */
   PRUint32 ComputeJustifiableCharacters(PRInt32 aOffset, PRInt32 aLength);
+  /**
+   * Find the start and end of the justifiable characters. Does not depend on the
+   * position of aStart or aEnd, although it's most efficient if they are near the
+   * start and end of the text frame.
+   */
   void FindJustificationRange(gfxSkipCharsIterator* aStart,
                               gfxSkipCharsIterator* aEnd);
 
   const nsStyleText* GetStyleText() { return mTextStyle; }
   nsTextFrame* GetFrame() { return mFrame; }
   // This may not be equal to the frame offset/length in because we may have
   // adjusted for whitespace trimming according to the state bits set in the frame
   // (for the static provider)
@@ -4878,17 +4883,16 @@ nsTextFrame::AddInlineMinWidthForFlow(ns
 
     if (i > wordStart) {
       nscoord width =
         NSToCoordCeil(mTextRun->GetAdvanceWidth(wordStart, i - wordStart, &provider));
       aData->currentLine += width;
       aData->atStartOfLine = PR_FALSE;
 
       if (collapseWhitespace) {
-        nscoord trailingWhitespaceWidth;
         PRUint32 trimStart = GetEndOfTrimmedText(frag, wordStart, i, &iter);
         if (trimStart == start) {
           // This is *all* trimmable whitespace, so whatever trailingWhitespace
           // we saw previously is still trailing...
           aData->trailingWhitespace += width;
         } else {
           // Some non-whitespace so the old trailingWhitespace is no longer trailing
           aData->trailingWhitespace =
@@ -5508,109 +5512,127 @@ nsTextFrame::Reflow(nsPresContext*      
 
 /* virtual */ PRBool
 nsTextFrame::CanContinueTextRun() const
 {
   // We can continue a text run through a text frame
   return PR_TRUE;
 }
 
-NS_IMETHODIMP
-nsTextFrame::TrimTrailingWhiteSpace(nsPresContext* aPresContext,
-                                    nsIRenderingContext& aRC,
-                                    nscoord& aDeltaWidth,
-                                    PRBool& aLastCharIsJustifiable)
-{
-  aLastCharIsJustifiable = PR_FALSE;
-  aDeltaWidth = 0;
+nsTextFrame::TrimOutput
+nsTextFrame::TrimTrailingWhiteSpace(nsIRenderingContext* aRC)
+{
+  TrimOutput result;
+  result.mChanged = PR_FALSE;
+  result.mLastCharIsJustifiable = PR_FALSE;
+  result.mDeltaWidth = 0;
 
   AddStateBits(TEXT_END_OF_LINE);
 
   PRInt32 contentLength = GetContentLength();
   if (!contentLength)
-    return NS_OK;
+    return result;
 
   gfxContext* ctx = static_cast<gfxContext*>
-    (aRC.GetNativeGraphicData(nsIRenderingContext::NATIVE_THEBES_CONTEXT));
+    (aRC->GetNativeGraphicData(nsIRenderingContext::NATIVE_THEBES_CONTEXT));
   gfxSkipCharsIterator start = EnsureTextRun(ctx);
-  if (!mTextRun)
-    return NS_ERROR_FAILURE;
+  NS_ENSURE_TRUE(mTextRun, result);
+
   PRUint32 trimmedStart = start.GetSkippedOffset();
 
   const nsTextFragment* frag = mContent->GetText();
   TrimmedOffsets trimmed = GetTrimmedOffsets(frag, PR_TRUE);
-  gfxSkipCharsIterator iter = start;
+  gfxSkipCharsIterator trimmedEndIter = start;
   const nsStyleText* textStyle = GetStyleText();
   gfxFloat delta = 0;
-  PRUint32 trimmedEnd = iter.ConvertOriginalToSkipped(trimmed.GetEnd());
+  PRUint32 trimmedEnd = trimmedEndIter.ConvertOriginalToSkipped(trimmed.GetEnd());
   
   if (GetStateBits() & TEXT_TRIMMED_TRAILING_WHITESPACE) {
-    aLastCharIsJustifiable = PR_TRUE;
+    // We pre-trimmed this frame, so the last character is justifiable
+    result.mLastCharIsJustifiable = PR_TRUE;
   } else if (trimmed.GetEnd() < GetContentEnd()) {
-    gfxSkipCharsIterator end = iter;
+    gfxSkipCharsIterator end = trimmedEndIter;
     PRUint32 endOffset = end.ConvertOriginalToSkipped(GetContentOffset() + contentLength);
     if (trimmedEnd < endOffset) {
       // We can't be dealing with tabs here ... they wouldn't be trimmed. So it's
       // OK to pass null for the line container.
       PropertyProvider provider(mTextRun, textStyle, frag, this, start, contentLength,
                                 nsnull, 0);
       delta = mTextRun->GetAdvanceWidth(trimmedEnd, endOffset - trimmedEnd, &provider);
       // non-compressed whitespace being skipped at end of line -> justifiable
       // XXX should we actually *count* justifiable characters that should be
       // removed from the overall count? I think so...
-      aLastCharIsJustifiable = PR_TRUE;
+      result.mLastCharIsJustifiable = PR_TRUE;
+      result.mChanged = PR_TRUE;
     }
   }
 
-  if (!aLastCharIsJustifiable &&
+  if (trimmed.GetEnd() == GetContentEnd() &&
+      HasSoftHyphenBefore(frag, mTextRun, trimmed.mStart, trimmedEndIter)) {
+    // This is a soft hyphen break.
+    // Fix up metrics to include hyphen
+    result.mChanged = PR_TRUE;
+    gfxTextRunCache::AutoTextRun hyphenTextRun(GetHyphenTextRun(mTextRun, ctx, this));
+    if (hyphenTextRun.get()) {
+      delta = -hyphenTextRun->GetAdvanceWidth(0, hyphenTextRun->GetLength(), nsnull);
+    }
+    AddStateBits(TEXT_HYPHEN_BREAK);
+  }
+
+  if (!result.mLastCharIsJustifiable &&
       NS_STYLE_TEXT_ALIGN_JUSTIFY == textStyle->mTextAlign) {
     // Check if any character in the last cluster is justifiable
     PropertyProvider provider(mTextRun, textStyle, frag, this, start, contentLength,
                               nsnull, 0);
     PRBool isCJK = IsChineseJapaneseLangGroup(this);
-    gfxSkipCharsIterator justificationStart(iter), justificationEnd(iter);
+    gfxSkipCharsIterator justificationStart(start), justificationEnd(trimmedEndIter);
     provider.FindJustificationRange(&justificationStart, &justificationEnd);
 
     PRInt32 i;
     for (i = justificationEnd.GetOriginalOffset(); i < trimmed.GetEnd(); ++i) {
       if (IsJustifiableCharacter(frag, i, isCJK)) {
-        aLastCharIsJustifiable = PR_TRUE;
+        result.mLastCharIsJustifiable = PR_TRUE;
       }
     }
   }
 
   gfxFloat advanceDelta;
   mTextRun->SetLineBreaks(trimmedStart, trimmedEnd - trimmedStart,
                           (GetStateBits() & TEXT_START_OF_LINE) != 0, PR_TRUE,
                           &advanceDelta, ctx);
+  if (advanceDelta != 0) {
+    result.mChanged = PR_TRUE;
+  }
 
   // aDeltaWidth is *subtracted* from our width.
   // If advanceDelta is positive then setting the line break made us longer,
   // so aDeltaWidth could go negative.
-  aDeltaWidth = NSToCoordFloor(delta - advanceDelta);
-  // XXX if aDeltaWidth goes negative, that means this frame might not actually fit
-  // anymore!!! We need higher level line layout to recover somehow. This can
-  // really only happen when we have glyphs with special shapes at the end of
-  // lines, I think. Breaking inside a kerning pair won't do it because that
-  // would mean we broke inside this textrun, and BreakAndMeasureText should
-  // make sure the resulting shaped substring fits. Maybe if we passed a
-  // maxTextLength? But that only happens at direction changes (so we
-  // we wouldn't kern across the boundary) or for first-letter (which always fits
-  // because it starts the line!).
-  if (aDeltaWidth < 0) {
-    NS_WARNING("Negative deltawidth, something odd is happening");
-  }
-
-  // XXX what about adjusting bounding metrics?
+  result.mDeltaWidth = NSToCoordFloor(delta - advanceDelta);
+  // If aDeltaWidth goes negative, that means this frame might not actually fit
+  // anymore!!! We need higher level line layout to recover somehow.
+  // If it's because the frame has a soft hyphen that is now being displayed,
+  // this should actually be OK, because our reflow recorded the break
+  // opportunity that allowed the soft hyphen to be used, and we wouldn't
+  // have recorded the opportunity unless the hyphen fit (or was the first
+  // opportunity on the line).
+  // Otherwise this can/ really only happen when we have glyphs with special
+  // shapes at the end of lines, I think. Breaking inside a kerning pair won't
+  // do it because that would mean we broke inside this textrun, and
+  // BreakAndMeasureText should make sure the resulting shaped substring fits.
+  // Maybe if we passed a maxTextLength? But that only happens at direction
+  // changes (so we wouldn't kern across the boundary) or for first-letter
+  // (which always fits because it starts the line!).
+  NS_WARN_IF_FALSE(result.mDeltaWidth >= 0 || (GetStateBits() & TEXT_HYPHEN_BREAK),
+                   "Negative deltawidth in a non-hyphen case, something odd is happening");
 
 #ifdef NOISY_TRIM
   ListTag(stdout);
-  printf(": trim => %d\n", aDeltaWidth);
+  printf(": trim => %d\n", result.mDeltaWidth);
 #endif
-  return NS_OK;
+  return result;
 }
 
 nsRect
 nsTextFrame::RecomputeOverflowRect()
 {
   gfxSkipCharsIterator iter = EnsureTextRun();
   if (!mTextRun)
     return nsRect(nsPoint(0,0), GetSize());
--- a/layout/reftests/reftest.list
+++ b/layout/reftests/reftest.list
@@ -43,16 +43,19 @@ include counters/reftest.list
 include first-letter/reftest.list
 
 # first-line/
 include first-line/reftest.list
 
 # svg/
 include svg/reftest.list
 
+# text/
+include text/reftest.list
+
 # text-decoration/
 include text-decoration/reftest.list
 
 # text-indent/
 include text-indent/reftest.list
 
 # text-transform/
 include text-transform/reftest.list
--- a/layout/reftests/text/soft-hyphens-1-ref.html
+++ b/layout/reftests/text/soft-hyphens-1-ref.html
@@ -1,13 +1,13 @@
 <html>
 <body>
 
 <div>
-<p>Hy-<br>phen.
-<p>Hy-<br>phen.
-<p>Hy-<br>phen.
-<p>Hy-<br>phen.
-<p>Hy-<br>phen.
+<p>Hy&#x2010;<br>phen.
+<p>Hy&#x2010;<br>phen.
+<p>Hy&#x2010;<br>phen.
+<p>Hy&#x2010;<br>phen.
+<p>Hy&#x2010;<br>phen.
 </div>
 
 </body>
 </html>