Not Part Of The Build. Update nsTextFrameThebes whitespace handling; allow line breaking only at the end of a run of whitespace.
authorroc+@cs.cmu.edu
Tue, 22 May 2007 16:45:47 -0700
changeset 1732 99baf0bab7dd978a2d35fa63cce7d1120913d7ec
parent 1731 eed3701cf4e4c9057ff8c6b7d80482b341a44f2d
child 1733 db21657f1dfaca7c715b97de5b9fc3cfa10ce14e
push id1
push userbsmedberg@mozilla.com
push dateThu, 20 Mar 2008 16:49:24 +0000
treeherdermozilla-central@61007906a1f8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
milestone1.9a5pre
Not Part Of The Build. Update nsTextFrameThebes whitespace handling; allow line breaking only at the end of a run of whitespace.
content/base/public/nsLineBreaker.h
content/base/src/nsLineBreaker.cpp
layout/generic/nsTextFrameThebes.cpp
layout/generic/nsTextFrameUtils.cpp
--- a/content/base/public/nsLineBreaker.h
+++ b/content/base/public/nsLineBreaker.h
@@ -90,21 +90,27 @@ public:
   // whitespace are always controlled by the
   // 'white-space' property of the text node containing the
   // whitespace. Breaks induced by non-whitespace where the break is between
   // two nodes are controled by the 'white-space' property on the nearest
   // common ancestor node. Therefore we provide separate control over
   // a) whether whitespace in this text induces breaks b) whether we can
   // break between nonwhitespace inside this text and c) whether we can break
   // between nonwhitespace between the last text and this text.
+  //
+  // "Whitespace" below means Unicode ZWSP (U+200B) and ASCII space (U+0020). We
+  // operate on text after whitespace processing has been applied, so
+  // other characters (e.g. tabs and newlines) may have been converted to
+  // spaces.
   enum {
     /**
-     * Allow breaks before and after whitespace in this block of text
+     * Allow breaks where a non-whitespace character in this block of text
+     * is preceded by a whitespace character.
      */
-    BREAK_WHITESPACE           = 0x01,
+    BREAK_WHITESPACE_END       = 0x01,
     /**
      * Allow breaks between eligible nonwhitespace characters when the break
      * is in the interior of this block of text.
      */
     BREAK_NONWHITESPACE_INSIDE = 0x02,
     /**
      * Allow break between eligible nonwhitespace characters when the break
      * is at the beginning of this block of text.
@@ -157,13 +163,13 @@ private:
   nsresult FlushCurrentWord();
 
   nsAutoTArray<PRUnichar,100> mCurrentWord;
   // All the items that contribute to mCurrentWord
   nsAutoTArray<TextItem,2>    mTextItems;
   PRPackedBool                mCurrentWordContainsCJK;
 
   // When mCurrentWord is empty, this indicates whether we should allow a break
-  // before the next text.
-  PRPackedBool             mBreakBeforeNextWord;
+  // before the next text if it starts with non-whitespace.
+  PRPackedBool                mBreakBeforeNonWhitespace;
 };
 
 #endif /*NSLINEBREAKER_H_*/
--- a/content/base/src/nsLineBreaker.cpp
+++ b/content/base/src/nsLineBreaker.cpp
@@ -35,44 +35,42 @@
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 #include "nsLineBreaker.h"
 #include "nsContentUtils.h"
 #include "nsILineBreaker.h"
 
-// We only operate on post-transformation text, so we don't see tabs
+#define UNICODE_ZWSP 0x200b
+
 static inline int
 IS_SPACE(PRUnichar u)
 {
-  NS_ASSERTION(u != 0x0009, "Tabs should have been eliminated");
-  // \r characters are just ignored
-  return u == 0x0020 || u == 0x000a || u == 0x200b;
+  return u == 0x0020 || u == UNICODE_ZWSP;
 }
 
 static inline int
 IS_SPACE(PRUint8 u)
 {
-  NS_ASSERTION(u != 0x0009, "Tabs should have been eliminated");
-  // \r characters are just ignored
-  return u == 0x0020 || u == 0x000a;
+  return u == 0x0020;
 }
 
 static inline int
 IS_CJK_CHAR(PRUnichar u)
 {
   return (0x1100 <= u && u <= 0x11ff) ||
          (0x2e80 <= u && u <= 0xd7ff) ||
          (0xf900 <= u && u <= 0xfaff) ||
          (0xff00 <= u && u <= 0xffef);
 }
 
 nsLineBreaker::nsLineBreaker()
-  : mCurrentWordContainsCJK(PR_FALSE), mBreakBeforeNextWord(PR_FALSE)
+  : mCurrentWordContainsCJK(PR_FALSE),
+    mBreakBeforeNonWhitespace(PR_FALSE)
 {
 }
 
 nsLineBreaker::~nsLineBreaker()
 {
   NS_ASSERTION(mCurrentWord.Length() == 0, "Should have Reset() before destruction!");
 }
 
@@ -124,25 +122,25 @@ nsresult
 nsLineBreaker::AppendText(nsIAtom* aLangGroup, const PRUnichar* aText, PRUint32 aLength,
                           PRUint32 aFlags, nsILineBreakSink* aSink)
 {
   if (aLength == 0) {
     // Treat as "invisible whitespace"
     nsresult rv = FlushCurrentWord();
     if (NS_FAILED(rv))
       return rv;
-    mBreakBeforeNextWord |= (aFlags & BREAK_WHITESPACE) != 0;
+    mBreakBeforeNonWhitespace = (aFlags & BREAK_WHITESPACE_END) != 0;
     return NS_OK;
   }
 
   PRUint32 offset = 0;
 
   // Continue the current word
   if (mCurrentWord.Length() > 0) {
-    NS_ASSERTION(!mBreakBeforeNextWord, "This should not be set");
+    NS_ASSERTION(!mBreakBeforeNonWhitespace, "These should not be set");
 
     while (offset < aLength && !IS_SPACE(aText[offset])) {
       mCurrentWord.AppendElement(aText[offset]);
       if (!mCurrentWordContainsCJK && IS_CJK_CHAR(aText[offset])) {
         mCurrentWordContainsCJK = PR_TRUE;
       }
       ++offset;
     }
@@ -163,42 +161,40 @@ nsLineBreaker::AppendText(nsIAtom* aLang
   nsAutoTArray<PRPackedBool,4000> breakState;
   if (!breakState.AppendElements(aLength))
     return NS_ERROR_OUT_OF_MEMORY;
 
   PRUint32 start = offset;
   PRUint32 wordStart = offset;
   PRBool wordHasCJK = PR_FALSE;
 
-  PRBool breakNext = mBreakBeforeNextWord;
+  PRBool breakNextIfNonWhitespace = mBreakBeforeNonWhitespace;
   for (;;) {
     PRUnichar ch = aText[offset];
     PRBool isSpace = IS_SPACE(ch);
 
-    breakState[offset] = breakNext;
-    breakNext = PR_FALSE;
+    breakState[offset] = breakNextIfNonWhitespace && !isSpace;
+    breakNextIfNonWhitespace = PR_FALSE;
 
     if (isSpace) {
       if (offset > wordStart && wordHasCJK) {
         if (aFlags & BREAK_NONWHITESPACE_INSIDE) {
           // Save current start-of-word state because GetJISx4051Breaks will
           // set it to false
           PRPackedBool currentStart = breakState[wordStart];
           nsContentUtils::LineBreaker()->
             GetJISx4051Breaks(aText + wordStart, offset - wordStart,
                               breakState.Elements() + wordStart);
           breakState[wordStart] = currentStart;
         }
         wordHasCJK = PR_FALSE;
       }
 
-      if (aFlags & BREAK_WHITESPACE) {
-        // Allow break either side of the whitespace
-        breakState[offset] = PR_TRUE;
-        breakNext = PR_TRUE;
+      if (aFlags & BREAK_WHITESPACE_END) {
+        breakNextIfNonWhitespace = PR_TRUE;
       }
       ++offset;
       if (offset >= aLength)
         break;
       wordStart = offset;
     } else {
       if (!wordHasCJK && IS_CJK_CHAR(ch)) {
         wordHasCJK = PR_TRUE;
@@ -216,38 +212,38 @@ nsLineBreaker::AppendText(nsIAtom* aLang
         // Ensure that the break-before for this word is written out
         offset = wordStart + 1;
         break;
       }
     }
   }
 
   aSink->SetBreaks(start, offset - start, breakState.Elements() + start);
-  mBreakBeforeNextWord = breakNext;
+  mBreakBeforeNonWhitespace = breakNextIfNonWhitespace;
   return NS_OK;
 }
 
 nsresult
 nsLineBreaker::AppendText(nsIAtom* aLangGroup, const PRUint8* aText, PRUint32 aLength,
                           PRUint32 aFlags, nsILineBreakSink* aSink)
 {
   if (aLength == 0) {
     // Treat as "invisible whitespace"
     nsresult rv = FlushCurrentWord();
     if (NS_FAILED(rv))
       return rv;
-    mBreakBeforeNextWord |= (aFlags & BREAK_WHITESPACE) != 0;
+    mBreakBeforeNonWhitespace = (aFlags & BREAK_WHITESPACE_END) != 0;
     return NS_OK;
   }
 
   PRUint32 offset = 0;
 
   // Continue the current word
   if (mCurrentWord.Length() > 0) {
-    NS_ASSERTION(!mBreakBeforeNextWord, "This should not be set");
+    NS_ASSERTION(!mBreakBeforeNonWhitespace, "These should not be set");
 
     while (offset < aLength && !IS_SPACE(aText[offset])) {
       mCurrentWord.AppendElement(aText[offset]);
       ++offset;
     }
 
     if (offset > 0) {
       mTextItems.AppendElement(TextItem(aSink, 0, offset, aFlags));
@@ -266,29 +262,27 @@ nsLineBreaker::AppendText(nsIAtom* aLang
 
   nsAutoTArray<PRPackedBool,4000> breakState;
   if (!breakState.AppendElements(aLength))
     return NS_ERROR_OUT_OF_MEMORY;
 
   PRUint32 start = offset;
   PRUint32 wordStart = offset;
 
-  PRBool breakNext = mBreakBeforeNextWord;
+  PRBool breakNextIfNonWhitespace = mBreakBeforeNonWhitespace;
   for (;;) {
     PRUint8 ch = aText[offset];
     PRBool isSpace = IS_SPACE(ch);
 
-    breakState[offset] = breakNext;
-    breakNext = PR_FALSE;
+    breakState[offset] = breakNextIfNonWhitespace && !isSpace;
+    breakNextIfNonWhitespace = PR_FALSE;
 
     if (isSpace) {
-      if (aFlags & BREAK_WHITESPACE) {
-        // Allow break either side of the whitespace
-        breakState[offset] = PR_TRUE;
-        breakNext = PR_TRUE;
+      if (aFlags & BREAK_WHITESPACE_END) {
+        breakNextIfNonWhitespace = PR_TRUE;
       }
       ++offset;
       if (offset >= aLength)
         break;
       wordStart = offset;
     } else {
       ++offset;
       if (offset >= aLength) {
@@ -308,11 +302,11 @@ nsLineBreaker::AppendText(nsIAtom* aLang
         break;
       }
       // We can't break inside words in 8-bit text (no CJK characters), so
       // there is no need to do anything else to handle words
     }
   }
 
   aSink->SetBreaks(start, offset - start, breakState.Elements() + start);
-  mBreakBeforeNextWord = breakNext;
+  mBreakBeforeNonWhitespace = breakNextIfNonWhitespace;
   return NS_OK;
 }
--- a/layout/generic/nsTextFrameThebes.cpp
+++ b/layout/generic/nsTextFrameThebes.cpp
@@ -170,29 +170,25 @@
  * A gfxTextRun can cover more than one DOM text node. This is necessary to
  * get kerning, ligatures and shaping for text that spans multiple text nodes
  * but is all the same font. The userdata for a gfxTextRun object is a
  * TextRunUserData* or an nsIFrame*.
  * 
  * We go to considerable effort to make sure things work even if in-flow
  * siblings have different style contexts (i.e., first-letter and first-line).
  * 
- * Tabs in preformatted text act as if they expand to 1-8 spaces, so that the
- * number of clusters on the line up to the end of the tab is a multiple of 8.
- * This is implemented by transforming tabs to spaces before handing off text
- * to the text run, and then configuring the spacing after the tab to be the
- * width of 0-7 spaces.
- * 
  * Our convention is that unsigned integer character offsets are offsets into
  * the transformed string. Signed integer character offsets are offsets into
  * the DOM string.
  * 
  * XXX currently we don't handle hyphenated breaks between text frames where the
  * hyphen occurs at the end of the first text frame, e.g.
  *   <b>Kit&shy;</b>ty
+ * 
+ * Preformatted text basically matches 
  */
 
 class nsTextFrame;
 class PropertyProvider;
 
 /**
  * We use an array of these objects to record which text frames
  * are associated with the textrun. mStartFrame is the start of a list of
@@ -522,18 +518,16 @@ public:
    */
   gfxSkipCharsIterator EnsureTextRun(nsIRenderingContext* aRC = nsnull,
                                      nsBlockFrame* aBlock = nsnull,
                                      const nsLineList::iterator* aLine = nsnull,
                                      PRUint32* aFlowEndInTextRun = nsnull);
 
   gfxTextRun* GetTextRun() { return mTextRun; }
   void SetTextRun(gfxTextRun* aTextRun) { mTextRun = aTextRun; }
-  
-  PRInt32 GetColumn() { return mColumn; }
 
   // Get the DOM content range mapped by this frame after excluding
   // whitespace subject to start-of-line and end-of-line trimming.
   // The textrun must have been created before calling this.
   struct TrimmedOffsets {
     PRInt32 mStart;
     PRInt32 mLength;
   };
@@ -541,17 +535,16 @@ public:
                                    PRBool aTrimAfter);
 
 protected:
   virtual ~nsTextFrame();
   
   nsIFrame*   mNextContinuation;
   PRInt32     mContentOffset;
   PRInt32     mContentLength;
-  PRInt32     mColumn;
   nscoord     mAscent;
   gfxTextRun* mTextRun;
 
   SelectionDetails* GetSelectionDetails();
   
   void AdjustSelectionPointsForBidi(SelectionDetails *sdptr,
                                     PRInt32 textLength,
                                     PRBool isRTLChars,
@@ -1553,21 +1546,25 @@ BuildTextRunsScanner::SetupBreakSinksFor
        : mMappedFlows[i + 1].mTransformedTextOffset)
       - offset;
 
     PRUint32 flags = 0;
     if (!mappedFlow->mAncestorControllingInitialBreak ||
         mappedFlow->mAncestorControllingInitialBreak->GetStyleText()->WhiteSpaceCanWrap()) {
       flags |= nsLineBreaker::BREAK_NONWHITESPACE_BEFORE;
     }
-    if (mappedFlow->mStartFrame->GetStyleText()->WhiteSpaceCanWrap()) {
-      flags |= nsLineBreaker::BREAK_WHITESPACE | nsLineBreaker::BREAK_NONWHITESPACE_INSIDE;
+    const nsStyleText* textStyle = mappedFlow->mStartFrame->GetStyleText();
+    if (textStyle->WhiteSpaceCanWrap()) {
+      // If white-space is preserved, then the only break opportunity is at
+      // the end of whitespace runs; otherwise there is a break opportunity before
+      // and after each whitespace character
+      flags |= nsLineBreaker::BREAK_NONWHITESPACE_INSIDE |
+        nsLineBreaker::BREAK_WHITESPACE_END;
     }
-    // If length is zero and BREAK_WHITESPACE is active, this will notify
-    // the linebreaker to insert a break opportunity before the next character.
+    // If length is zero, the linebreaker treats the text as invisible whitespace.
     // Thus runs of entirely-skipped whitespace can still induce breaks.
     if (aTextRun->GetFlags() & gfxFontGroup::TEXT_IS_8BIT) {
       mLineBreaker.AppendText(lang, aTextRun->GetText8Bit() + offset,
                               length, flags, *breakSink);
     } else {
       mLineBreaker.AppendText(lang, aTextRun->GetTextUnicode() + offset,
                               length, flags, *breakSink);
     }
@@ -1836,98 +1833,107 @@ static PRBool IsInBounds(const gfxSkipCh
 class PropertyProvider : public gfxTextRun::PropertyProvider {
 public:
   /**
    * Use this constructor for reflow, when we don't know what text is
    * really mapped by the frame and we have a lot of other data around.
    */
   PropertyProvider(gfxTextRun* aTextRun, const nsStyleText* aTextStyle,
                    const nsTextFragment* aFrag, nsTextFrame* aFrame,
-                   const gfxSkipCharsIterator& aStart, PRInt32 aLength)
+                   const gfxSkipCharsIterator& aStart, PRInt32 aLength,
+                   nsIFrame* aLineContainer,
+                   nscoord aOffsetFromBlockOriginForTabs)
     : mTextRun(aTextRun), mFontGroup(nsnull), mTextStyle(aTextStyle), mFrag(aFrag),
-      mFrame(aFrame), mStart(aStart), mLength(aLength),
+      mLineContainer(aLineContainer),
+      mFrame(aFrame), mStart(aStart), mTabWidths(nsnull), mLength(aLength),
       mWordSpacing(StyleToCoord(mTextStyle->mWordSpacing)),
       mLetterSpacing(StyleToCoord(mTextStyle->mLetterSpacing)),
       mJustificationSpacing(0),
-      mHyphenWidth(-1)
+      mHyphenWidth(-1),
+      mOffsetFromBlockOriginForTabs(aOffsetFromBlockOriginForTabs),
+      mReflowing(PR_TRUE)
   {
     NS_ASSERTION(mStart.IsInitialized(), "Start not initialized?");
   }
 
   /**
    * Use this constructor after the frame has been reflowed and we don't
    * have other data around. Gets everything from the frame. EnsureTextRun
    * *must* be called before this!!!
    */
   PropertyProvider(nsTextFrame* aFrame, const gfxSkipCharsIterator& aStart)
-    : mTextRun(aFrame->GetTextRun()), mFontGroup(nsnull), mTextStyle(aFrame->GetStyleText()),
+    : mTextRun(aFrame->GetTextRun()), mFontGroup(nsnull),
+      mTextStyle(aFrame->GetStyleText()),
       mFrag(aFrame->GetContent()->GetText()),
-      mFrame(aFrame), mStart(aStart), mLength(aFrame->GetContentLength()),
+      mLineContainer(nsnull),
+      mFrame(aFrame), mStart(aStart), mTabWidths(nsnull),
+      mLength(aFrame->GetContentLength()),
       mWordSpacing(StyleToCoord(mTextStyle->mWordSpacing)),
       mLetterSpacing(StyleToCoord(mTextStyle->mLetterSpacing)),
       mJustificationSpacing(0),
-      mHyphenWidth(-1)
+      mHyphenWidth(-1),
+      mOffsetFromBlockOriginForTabs(0),
+      mReflowing(PR_FALSE)
   {
     NS_ASSERTION(mTextRun, "Textrun not initialized!");
   }
 
   // Call this after construction if you're not going to reflow the text
   void InitializeForDisplay(PRBool aTrimAfter);
 
   virtual void GetSpacing(PRUint32 aStart, PRUint32 aLength, Spacing* aSpacing);
   virtual gfxFloat GetHyphenWidth();
   virtual void GetHyphenationBreaks(PRUint32 aStart, PRUint32 aLength,
                                     PRPackedBool* aBreakBefore);
 
+  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);
   void FindEndOfJustificationRange(gfxSkipCharsIterator* aIter);
 
-  /**
-   * Count the number of spaces inserted for tabs in the given transformed substring
-   */
-  PRUint32 GetTabExpansionCount(PRUint32 aOffset, PRUint32 aLength);
-
   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)
   const gfxSkipCharsIterator& GetStart() { return mStart; }
   PRUint32 GetOriginalLength() { return mLength; }
   const nsTextFragment* GetFragment() { return mFrag; }
 
   gfxFontGroup* GetFontGroup() {
     if (!mFontGroup) {
       mFontGroup = GetFontGroupForFrame(mFrame);
     }
     return mFontGroup;
   }
 
+  gfxFloat* GetTabWidths(PRUint32 aTransformedStart, PRUint32 aTransformedLength);
+
 protected:
   void SetupJustificationSpacing();
-
-  // Offsets in transformed string coordinates
-  PRUint8* ComputeTabSpaceCount(PRUint32 aOffset, PRUint32 aLength);
-
+  
   gfxTextRun*           mTextRun;
   gfxFontGroup*         mFontGroup;
   const nsStyleText*    mTextStyle;
   const nsTextFragment* mFrag;
+  nsIFrame*             mLineContainer;
   nsTextFrame*          mFrame;
   gfxSkipCharsIterator  mStart;  // Offset in original and transformed string
-  nsTArray<PRUint8>     mTabSpaceCounts;  // counts for transformed string characters
-  PRUint32              mCurrentColumn;
+  nsTArray<gfxFloat>*   mTabWidths;  // widths for each transformed string character
   PRInt32               mLength; // DOM string length
   gfxFloat              mWordSpacing;     // space for each whitespace char
   gfxFloat              mLetterSpacing;   // space for each letter
   gfxFloat              mJustificationSpacing;
   gfxFloat              mHyphenWidth;
+  gfxFloat              mOffsetFromBlockOriginForTabs;
+  PRPackedBool          mReflowing;
 };
 
 PRUint32
 PropertyProvider::ComputeJustifiableCharacters(PRInt32 aOffset, PRInt32 aLength)
 {
   // Scan non-skipped characters and count justifiable chars.
   nsSkipCharsRunIterator
     run(mStart, nsSkipCharsRunIterator::LENGTH_INCLUDES_SKIPPED, aLength);
@@ -1939,52 +1945,16 @@ PropertyProvider::ComputeJustifiableChar
     for (i = 0; i < run.GetRunLength(); ++i) {
       justifiableChars +=
         IsJustifiableCharacter(mFrag, run.GetOriginalOffset() + i, isCJK);
     }
   }
   return justifiableChars;
 }
 
-PRUint8*
-PropertyProvider::ComputeTabSpaceCount(PRUint32 aOffset, PRUint32 aLength)
-{
-  PRUint32 tabsEnd = mStart.GetSkippedOffset() + mTabSpaceCounts.Length();
-  // We incrementally compute the tab space counts because it could be
-  // inefficient to run ahead and compute space counts for tabs we don't need.
-  // mTabSpaceCounts is an array of space counts whose first element is
-  // the space count for the character at startOffset
-  if (aOffset + aLength > tabsEnd) {
-    PRUint32 column = mTabSpaceCounts.Length() ? mCurrentColumn : mFrame->GetColumn();
-    PRInt32 count = aOffset + aLength - tabsEnd;
-    nsSkipCharsRunIterator
-      run(mStart, nsSkipCharsRunIterator::LENGTH_UNSKIPPED_ONLY, count);
-    run.SetSkippedOffset(tabsEnd);
-    while (run.NextRun()) {
-      PRInt32 i;
-      for (i = 0; i < run.GetRunLength(); ++i) {
-        if (mFrag->CharAt(i + run.GetOriginalOffset()) == '\t') {
-          PRInt32 spaces = 8 - column%8;
-          column += spaces;
-          // The tab itself counts as a space
-          mTabSpaceCounts.AppendElement(spaces - 1);
-        } else {
-          if (mTextRun->IsClusterStart(i + run.GetSkippedOffset())) {
-            ++column;
-          }
-          mTabSpaceCounts.AppendElement(0);
-        }
-      }
-    }
-    mCurrentColumn = column;
-  }
-
-  return mTabSpaceCounts.Elements() + aOffset - mStart.GetSkippedOffset();
-}
-
 /**
  * Finds the offset of the first character of the cluster containing aPos
  */
 static void FindClusterStart(gfxTextRun* aTextRun,
                              gfxSkipCharsIterator* aPos)
 {
   while (aPos->GetOriginalOffset() > 0) {
     if (aPos->IsOriginalCharSkipped() ||
@@ -2014,16 +1984,24 @@ static void FindClusterEnd(gfxTextRun* a
   aPos->AdvanceOriginal(-1);
 }
 
 // aStart, aLength in transformed string offsets
 void
 PropertyProvider::GetSpacing(PRUint32 aStart, PRUint32 aLength,
                              Spacing* aSpacing)
 {
+  GetSpacingInternal(aStart, aLength, aSpacing,
+                     (mTextRun->GetFlags() & nsTextFrameUtils::TEXT_HAS_TAB) == 0);
+}
+
+void
+PropertyProvider::GetSpacingInternal(PRUint32 aStart, PRUint32 aLength,
+                                     Spacing* aSpacing, PRBool aIgnoreTabs)
+{
   NS_PRECONDITION(IsInBounds(mStart, mLength, aStart, aLength), "Range out of bounds");
 
   PRUint32 index;
   for (index = 0; index < aLength; ++index) {
     aSpacing[index].mBefore = 0.0;
     aSpacing[index].mAfter = 0.0;
   }
 
@@ -2054,26 +2032,22 @@ PropertyProvider::GetSpacing(PRUint32 aS
                          &iter);
           aSpacing[iter.GetSkippedOffset() - aStart].mAfter += mWordSpacing;
         }
       }
     }
   }
 
   // Now add tab spacing, if there is any
-  if (mTextRun->GetFlags() & nsTextFrameUtils::TEXT_HAS_TAB) {
-    // ComputeTabSpaceCount() will tell us where spaces need to be inserted
-    // for tabs, and how many.
-    // ComputeTabSpaceCount takes transformed string offsets.
-    PRUint8* tabSpaceList = ComputeTabSpaceCount(aStart, aLength);
-    gfxFloat spaceWidth = mLetterSpacing + mWordSpacing +
-        mTextRun->GetFontGroup()->GetFontAt(0)->GetMetrics().spaceWidth*mTextRun->GetAppUnitsPerDevUnit();
-    for (index = 0; index < aLength; ++index) {
-      PRInt32 tabSpaces = tabSpaceList[index];
-      aSpacing[index].mAfter += spaceWidth*tabSpaces;
+  if (!aIgnoreTabs) {
+    gfxFloat* tabs = GetTabWidths(aStart, aLength);
+    if (tabs) {
+      for (index = 0; index < aLength; ++index) {
+        aSpacing[index].mAfter += tabs[index];
+      }
     }
   }
 
   // Now add in justification spacing
   if (mJustificationSpacing) {
     gfxFloat halfJustificationSpace = mJustificationSpacing/2;
     // Scan non-skipped characters and adjust justifiable chars, adding
     // justification space on either side of the cluster
@@ -2100,29 +2074,101 @@ PropertyProvider::GetSpacing(PRUint32 aS
             aSpacing[clusterLastChar - aStart].mAfter += halfJustificationSpace;
           }
         }
       }
     }
   }
 }
 
-PRUint32
-PropertyProvider::GetTabExpansionCount(PRUint32 aStart, PRUint32 aLength)
+static void TabWidthDestructor(void* aObject, nsIAtom* aProp, void* aValue,
+                               void* aData)
+{
+  delete NS_STATIC_CAST(nsTArray<gfxFloat>*, aValue);
+}
+
+gfxFloat*
+PropertyProvider::GetTabWidths(PRUint32 aStart, PRUint32 aLength)
 {
-  if (!(mTextRun->GetFlags() & nsTextFrameUtils::TEXT_HAS_TAB))
-    return 0;
-
-  PRUint8* spaces = ComputeTabSpaceCount(aStart, aLength);
-  PRUint32 i;
-  PRUint32 sum = 0;
-  for (i = 0; i < aLength; ++i) {
-    sum += spaces[i];
-  }
-  return sum;
+  if (!mTabWidths) {
+    if (!mReflowing) {
+      mTabWidths = NS_STATIC_CAST(nsTArray<gfxFloat>*,
+        mFrame->GetProperty(nsGkAtoms::tabWidthProperty));
+      if (!mTabWidths) {
+        NS_WARNING("We need precomputed tab widths, but they're not here...");
+        return nsnull;
+      }
+    } else {
+      nsAutoPtr<nsTArray<gfxFloat> > tabs(new nsTArray<gfxFloat>());
+      if (!tabs)
+        return nsnull;
+      nsresult rv = mFrame->SetProperty(nsGkAtoms::tabWidthProperty, tabs,
+                                        TabWidthDestructor, nsnull);
+      if (NS_FAILED(rv))
+        return nsnull;
+      mTabWidths = tabs.forget();
+    }
+  }
+
+  PRUint32 startOffset = mStart.GetSkippedOffset();
+  PRUint32 tabsEnd = startOffset + mTabWidths->Length();
+  if (tabsEnd < aStart + aLength) {
+    if (!mReflowing) {
+      NS_WARNING("We need precomputed tab widths, but we don't have enough...");
+      return nsnull;
+    }
+    
+    if (!mTabWidths->AppendElements(aStart + aLength - tabsEnd))
+      return nsnull;
+    
+    PRUint32 i;
+    if (!mLineContainer) {
+      NS_WARNING("Tabs encountered in a situation where we don't support tabbing");
+      for (i = tabsEnd; i < aStart + aLength; ++i) {
+        (*mTabWidths)[i - startOffset] = 0;
+      }
+    } else {
+      gfxFloat tabWidth = NS_round(8*mTextRun->GetAppUnitsPerDevUnit()*
+        GetFontMetrics(GetFontGroupForFrame(mLineContainer)).spaceWidth);
+      
+      for (i = tabsEnd; i < aStart + aLength; ++i) {
+        Spacing spacing;
+        GetSpacingInternal(i, 1, &spacing, PR_TRUE);
+        mOffsetFromBlockOriginForTabs += spacing.mBefore;
+  
+        if (mTextRun->GetChar(i) != '\t') {
+          (*mTabWidths)[i - startOffset] = 0;
+          if (mTextRun->IsClusterStart(i)) {
+            PRUint32 clusterEnd = i + 1;
+            while (clusterEnd < mTextRun->GetLength() &&
+                   !mTextRun->IsClusterStart(clusterEnd)) {
+              ++clusterEnd;
+            }
+            mOffsetFromBlockOriginForTabs +=
+              mTextRun->GetAdvanceWidth(i, clusterEnd - i, nsnull);
+          }
+        } else {
+          // Advance mOffsetFromBlockOriginForTabs to the next multiple of tabWidth
+          // Ensure that if it's just epsilon less than a multiple of tabWidth, we still
+          // advance by tabWidth.
+          static const double EPSILON = 0.000001;
+          double nextTab = NS_ceil(mOffsetFromBlockOriginForTabs/tabWidth)*tabWidth;
+          if (nextTab < mOffsetFromBlockOriginForTabs + EPSILON) {
+            nextTab += tabWidth;
+          }
+          (*mTabWidths)[i - startOffset] = nextTab - mOffsetFromBlockOriginForTabs;
+          mOffsetFromBlockOriginForTabs = nextTab;
+        }
+  
+        mOffsetFromBlockOriginForTabs += spacing.mAfter;
+      }
+    }
+  }
+
+  return mTabWidths->Elements() + aStart - startOffset;
 }
 
 gfxFloat
 PropertyProvider::GetHyphenWidth()
 {
   if (mHyphenWidth < 0) {
     nsCOMPtr<nsIRenderingContext> rc = GetReferenceRenderingContext(mFrame, nsnull);
     gfxTextRun* hyphenTextRun = GetHyphenTextRun(mTextRun, rc);
@@ -4127,60 +4173,61 @@ nsTextFrame::GetPointFromOffset(nsPresCo
     outPoint->x = width;
   }
   outPoint->y = 0;
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
-nsTextFrame::GetChildFrameContainingOffset(PRInt32 inContentOffset,
-                                           PRBool  inHint,
-                                           PRInt32* outFrameContentOffset,
-                                           nsIFrame **outChildFrame)
+nsTextFrame::GetChildFrameContainingOffset(PRInt32   aContentOffset,
+                                           PRBool    aHint,
+                                           PRInt32*  aOutOffset,
+                                           nsIFrame**aOutFrame)
 {
   DEBUG_VERIFY_NOT_DIRTY(mState);
 #if 0 //XXXrbs disable due to bug 310227
   if (mState & NS_FRAME_IS_DIRTY)
     return NS_ERROR_UNEXPECTED;
 #endif
 
-  if (nsnull == outChildFrame)
-    return NS_ERROR_NULL_POINTER;
-  PRInt32 contentOffset = inContentOffset;
-  
-  if (contentOffset != -1) //-1 signified the end of the current content
-    contentOffset = inContentOffset - mContentOffset;
-
-  if ((contentOffset > mContentLength) || ((contentOffset == mContentLength) && inHint) )
-  {
-    //this is not the frame we are looking for.
-    nsIFrame* nextContinuation = GetNextContinuation();
-    if (nextContinuation)
-    {
-      return nextContinuation->GetChildFrameContainingOffset(inContentOffset, inHint, outFrameContentOffset, outChildFrame);
+  NS_ASSERTION(aOutOffset && aOutFrame, "Bad out parameters");
+  NS_ASSERTION(aContentOffset >= 0, "Negative content offset, existing code was very broken!");
+
+  nsTextFrame* f = this;
+  if (aContentOffset >= mContentOffset) {
+    while (PR_TRUE) {
+      nsTextFrame* next = NS_STATIC_CAST(nsTextFrame*, f->GetNextContinuation());
+      if (!next || aContentOffset < next->GetContentOffset())
+        break;
+      if (aContentOffset == next->GetContentOffset()) {
+        if (aHint) {
+          f = next;
+        }
+        break;
+      }
+      f = next;
     }
-    else {
-      if (contentOffset != mContentLength) //that condition was only for when there is a choice
-        return NS_ERROR_FAILURE;
+  } else {
+    while (PR_TRUE) {
+      nsTextFrame* prev = NS_STATIC_CAST(nsTextFrame*, f->GetPrevContinuation());
+      if (!prev || aContentOffset > f->GetContentOffset())
+        break;
+      if (aContentOffset == f->GetContentOffset()) {
+        if (!aHint) {
+          f = prev;
+        }
+        break;
+      }
+      f = prev;
     }
   }
-
-  if (inContentOffset < mContentOffset) //could happen with floats!
-  {
-    *outChildFrame = GetPrevInFlow();
-    if (*outChildFrame)
-      return (*outChildFrame)->GetChildFrameContainingOffset(inContentOffset, inHint,
-        outFrameContentOffset,outChildFrame);
-    else
-      return NS_OK; //this can't be the right thing to do?
-  }
   
-  *outFrameContentOffset = contentOffset;
-  *outChildFrame = this;
+  *aOutOffset = aContentOffset - f->GetContentOffset();
+  *aOutFrame = f;
   return NS_OK;
 }
 
 PRBool
 nsTextFrame::PeekOffsetNoAmount(PRBool aForward, PRInt32* aOffset)
 {
   NS_ASSERTION(aOffset && *aOffset <= mContentLength, "aOffset out of range");
 
@@ -4427,18 +4474,20 @@ nsTextFrame::AddInlineMinWidthForFlow(ns
                                       nsIFrame::InlineMinWidthData *aData)
 {
   PRUint32 flowEndInTextRun;
   gfxSkipCharsIterator iter =
     EnsureTextRun(aRenderingContext, nsnull, nsnull, &flowEndInTextRun);
   if (!mTextRun)
     return;
 
+  // Pass null for the line container. This will disable tab spacing, but that's
+  // OK since we can't really handle tabs for intrinsic sizing anyway.
   PropertyProvider provider(mTextRun, GetStyleText(), mContent->GetText(), this,
-                            iter, GetInFlowContentLength());
+                            iter, GetInFlowContentLength(), nsnull, 0);
 
   PRBool collapseWhitespace = !provider.GetStyleText()->WhiteSpaceIsSignificant();
   PRUint32 start =
     FindStartAfterSkippingWhitespace(&provider, aData, collapseWhitespace,
                                      &iter, flowEndInTextRun);
   if (start >= flowEndInTextRun)
     return;
 
@@ -4513,18 +4562,20 @@ nsTextFrame::AddInlinePrefWidthForFlow(n
                                        nsIFrame::InlinePrefWidthData *aData)
 {
   PRUint32 flowEndInTextRun;
   gfxSkipCharsIterator iter =
     EnsureTextRun(aRenderingContext, nsnull, nsnull, &flowEndInTextRun);
   if (!mTextRun)
     return;
 
+  // Pass null for the line container. This will disable tab spacing, but that's
+  // OK since we can't really handle tabs for intrinsic sizing anyway.
   PropertyProvider provider(mTextRun, GetStyleText(), mContent->GetText(), this,
-                            iter, GetInFlowContentLength());
+                            iter, GetInFlowContentLength(), nsnull, 0);
 
   PRBool collapseWhitespace = !provider.GetStyleText()->WhiteSpaceIsSignificant();
   PRUint32 start =
     FindStartAfterSkippingWhitespace(&provider, aData, collapseWhitespace,
                                      &iter, flowEndInTextRun);
   if (start >= flowEndInTextRun)
     return;
 
@@ -4702,19 +4753,16 @@ nsTextFrame::Reflow(nsPresContext*      
 
   const nsStyleText* textStyle = GetStyleText();
 
   PRBool atStartOfLine = lineLayout.CanPlaceFloatNow();
   if (atStartOfLine) {
     AddStateBits(TEXT_START_OF_LINE);
   }
 
-  PRInt32 column = lineLayout.GetColumn();
-  mColumn = column;
-
   // 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) {
     // Nuke any text run since it may not be valid for our reflow
     ClearTextRun();
   }
@@ -4743,16 +4791,17 @@ nsTextFrame::Reflow(nsPresContext*      
   PRInt32 offset = mContentOffset;
 
   // Restrict preformatted text to the nearest newline
   PRInt32 newLineOffset = -1;
   if (textStyle->WhiteSpaceIsSignificant()) {
     newLineOffset = FindChar(frag, offset, length, '\n');
     if (newLineOffset >= 0) {
       length = newLineOffset + 1 - offset;
+      newLineOffset -= mContentOffset;
     }
   } else {
     if (atStartOfLine) {
       // Skip leading whitespace
       PRInt32 whitespaceCount = GetWhitespaceCount(frag, offset, length, 1);
       offset += whitespaceCount;
       length -= whitespaceCount;
     }
@@ -4765,17 +4814,20 @@ nsTextFrame::Reflow(nsPresContext*      
     completedFirstLetter = FindFirstLetterRange(frag, mTextRun, offset, &length);
   }
 
   /////////////////////////////////////////////////////////////////////
   // See how much text should belong to this text frame, and measure it
   /////////////////////////////////////////////////////////////////////
   
   iter.SetOriginalOffset(offset);
-  PropertyProvider provider(mTextRun, textStyle, frag, this, iter, length);
+  nscoord xOffsetForTabs = (mTextRun->GetFlags() & nsTextFrameUtils::TEXT_HAS_TAB) ?
+         lineLayout.GetCurrentFrameXDistanceFromBlock() : -1;
+  PropertyProvider provider(mTextRun, textStyle, frag, this, iter, length,
+      lineContainer, xOffsetForTabs);
 
   PRUint32 transformedOffset = provider.GetStart().GetSkippedOffset();
 
   // The metrics for the text go in here
   gfxTextRun::Metrics textMetrics;
   PRBool needTightBoundingBox = (GetStateBits() & TEXT_FIRST_LETTER) != 0;
 #ifdef MOZ_MATHML
   if (NS_REFLOW_CALC_BOUNDING_METRICS & aMetrics.mFlags) {
@@ -4817,27 +4869,32 @@ nsTextFrame::Reflow(nsPresContext*      
     // we're not looking at all the content, so we need to compute the
     // length of the transformed substring we're looking at
     gfxSkipCharsIterator iter(provider.GetStart());
     iter.SetOriginalOffset(offset + limitLength);
     transformedLength = iter.GetSkippedOffset() - transformedOffset;
   }
   PRUint32 transformedLastBreak = 0;
   PRBool usedHyphenation;
+  gfxFloat trimmedWidth = 0;
+  gfxFloat availWidth = aReflowState.availableWidth;
+  PRBool canTrimTrailingWhitespace = !textStyle->WhiteSpaceIsSignificant();
   PRUint32 transformedCharsFit =
     mTextRun->BreakAndMeasureText(transformedOffset, transformedLength,
                                   (GetStateBits() & TEXT_START_OF_LINE) != 0,
-                                  aReflowState.availableWidth,
+                                  availWidth,
                                   &provider, suppressInitialBreak,
+                                  canTrimTrailingWhitespace ? &trimmedWidth : nsnull,
                                   &textMetrics, needTightBoundingBox,
                                   &usedHyphenation, &transformedLastBreak);
   end.SetSkippedOffset(transformedOffset + transformedCharsFit);
   PRInt32 charsFit = end.GetOriginalOffset() - offset;
-  // That might have taken us beyond our assigned content range, so get back
-  // in.
+  // That might have taken us beyond our assigned content range (because
+  // we might have advanced over some skipped chars that extend outside
+  // this frame), so get back in.
   PRInt32 lastBreak = -1;
   if (charsFit >= limitLength) {
     charsFit = limitLength;
     if (transformedLastBreak != PR_UINT32_MAX) {
       // lastBreak is needed. Use the "end" iterator for this because
       // it's likely to be close to the desired point
       end.SetSkippedOffset(transformedOffset + transformedLastBreak);
       // This may set lastBreak greater than 'length', but that's OK
@@ -4851,51 +4908,37 @@ nsTextFrame::Reflow(nsPresContext*      
     gfxTextRun* hyphenTextRun = GetHyphenTextRun(mTextRun, aReflowState.rendContext);
     if (hyphenTextRun) {
       AddCharToMetrics(hyphenTextRun,
                        mTextRun, &textMetrics, needTightBoundingBox);
     }
     AddStateBits(TEXT_HYPHEN_BREAK);
   }
 
-  // If it's only whitespace that didn't fit, then we will include
-  // the whitespace in this frame, but we will eagerly trim all trailing
-  // whitespace from our width calculations. Basically we do
-  // TrimTrailingWhitespace early so that line layout sees that we fit on
-  // the line.
-  if (charsFit < length) {
-    PRInt32 whitespaceCount =
-      GetWhitespaceCount(frag, offset + charsFit, length - charsFit, 1);
-    if (whitespaceCount > 0) {
-      charsFit += whitespaceCount;
-
-      // Now trim all trailing whitespace from our width, including possibly
-      // even whitespace that fitted (which could only happen with
-      // white-space:pre-wrap, because when whitespace collapsing is in effect
-      // there can only be one whitespace character rendered at the end of
-      // the frame)
-      AddStateBits(TEXT_TRIMMED_TRAILING_WHITESPACE);
-      PRUint32 currentEnd = end.GetSkippedOffset();
-      PRUint32 trimmedEnd = transformedOffset +
-        GetLengthOfTrimmedText(frag, transformedOffset, currentEnd, &end);
-      textMetrics.mAdvanceWidth -=
-        mTextRun->GetAdvanceWidth(trimmedEnd,
-                                  currentEnd - trimmedEnd, &provider);
-      // XXX We don't adjust the bounding box in textMetrics. But we should! Do
-      // we need to remeasure the text? Maybe the metrics returned by the textrun
-      // should have a way of saying "no glyphs outside their font-boxes" so
-      // we know we don't need to adjust the bounding box here and elsewhere...
-      end.SetOriginalOffset(offset + charsFit);
+  // If everything fits including trimmed whitespace, then we should add the
+  // trimmed whitespace to our metrics now because it probably won't be trimmed
+  // and we need to position subsequent frames correctly...
+  if (textMetrics.mAdvanceWidth + trimmedWidth <= availWidth) {
+    textMetrics.mAdvanceWidth += trimmedWidth;
+    if (mTextRun->IsRightToLeft()) {
+      // Space comes before text, so the bounding box is moved to the
+      // right by trimmdWidth
+      textMetrics.mBoundingBox.MoveBy(gfxPoint(trimmedWidth, 0));
     }
-  } else {
+    
     // All text fit. Record the last potential break, if there is one.
     if (lastBreak >= 0) {
       lineLayout.NotifyOptionalBreakPosition(mContent, lastBreak,
           textMetrics.mAdvanceWidth < aReflowState.availableWidth);
     }
+  } else {
+    // We're definitely going to break and our whitespace will definitely
+    // be trimmed.
+    // Record that whitespace has already been trimmed.
+    AddStateBits(TEXT_TRIMMED_TRAILING_WHITESPACE);
   }
   mContentLength = offset + charsFit - mContentOffset;
 
   /////////////////////////////////////////////////////////////////////
   // Compute output metrics
   /////////////////////////////////////////////////////////////////////
 
   // first-letter frames should use the tight bounding box metrics for ascent/descent
@@ -4936,21 +4979,16 @@ nsTextFrame::Reflow(nsPresContext*      
     aMetrics.mBoundingMetrics.width = aMetrics.width;
   }
 #endif
 
   /////////////////////////////////////////////////////////////////////
   // Clean up, update state
   /////////////////////////////////////////////////////////////////////
 
-  // Update lineLayout state  
-  column += textMetrics.mClusterCount +
-      provider.GetTabExpansionCount(provider.GetStart().GetSkippedOffset(),
-                                    GetSkippedDistance(provider.GetStart(), end));
-  lineLayout.SetColumn(column);
   lineLayout.SetUnderstandsWhiteSpace(PR_TRUE);
   PRBool endsInWhitespace = PR_FALSE;
   if (charsFit > 0) {
     endsInWhitespace = IsSpace(frag, offset + charsFit - 1);
     lineLayout.SetInWord(!endsInWhitespace);
     lineLayout.SetEndsInWhiteSpace(endsInWhitespace);
     PRBool wrapping = textStyle->WhiteSpaceCanWrap();
     lineLayout.SetTrailingTextFrame(this, wrapping);
@@ -5045,29 +5083,33 @@ nsTextFrame::TrimTrailingWhiteSpace(nsPr
   gfxFloat delta = 0;
 
   if (GetStateBits() & TEXT_TRIMMED_TRAILING_WHITESPACE) {
     aLastCharIsJustifiable = PR_TRUE;
   } else if (trimmed.mStart + trimmed.mLength < mContentOffset + mContentLength) {
     gfxSkipCharsIterator end = iter;
     PRUint32 endOffset = end.ConvertOriginalToSkipped(mContentOffset + mContentLength);
     if (trimmedEnd < endOffset) {
-      PropertyProvider provider(mTextRun, textStyle, frag, this, iter, mContentLength);
+      // 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, iter, mContentLength,
+                                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;
     }
   }
 
   if (!aLastCharIsJustifiable &&
       NS_STYLE_TEXT_ALIGN_JUSTIFY == textStyle->mTextAlign) {
     // Check if any character in the last cluster is justifiable
-    PropertyProvider provider(mTextRun, textStyle, frag, this, iter, mContentLength);
+    PropertyProvider provider(mTextRun, textStyle, frag, this, iter, mContentLength,
+                              nsnull, 0);
     PRBool isCJK = IsChineseJapaneseLangGroup(this);
     gfxSkipCharsIterator justificationEnd(iter);
     provider.FindEndOfJustificationRange(&justificationEnd);
 
     PRInt32 i;
     for (i = justificationEnd.GetOriginalOffset(); i < trimmed.mStart + trimmed.mLength; ++i) {
       if (IsJustifiableCharacter(frag, i, isCJK)) {
         aLastCharIsJustifiable = PR_TRUE;
--- a/layout/generic/nsTextFrameUtils.cpp
+++ b/layout/generic/nsTextFrameUtils.cpp
@@ -102,31 +102,29 @@ nsTextFrameUtils::TransformText(const PR
                                 PRPackedBool* aIncomingWhitespace,
                                 gfxSkipCharsBuilder* aSkipChars,
                                 PRUint32* aAnalysisFlags)
 {
   PRUint32 flags = 0;
   PRUnichar* outputStart = aOutput;
 
   if (!aCompressWhitespace) {
-    // Convert tabs and formfeeds to spaces and skip discardables.
+    // Skip discardables.
     PRUint32 i;
     for (i = 0; i < aLength; ++i) {
       PRUnichar ch = *aText++;
-      if (ch == '\t') {
-        flags |= TEXT_HAS_TAB|TEXT_WAS_TRANSFORMED;
-        aSkipChars->KeepChar();
-        *aOutput++ = ' ';
-      } else if (IsDiscardable(ch, &flags)) {
+      if (IsDiscardable(ch, &flags)) {
         aSkipChars->SkipChar();
       } else {
         aSkipChars->KeepChar();
         if (ch == CH_NBSP) {
           ch = ' ';
           flags |= TEXT_WAS_TRANSFORMED;
+        } else if (ch == '\t') {
+          flags |= TEXT_HAS_TAB;
         }
         *aOutput++ = ch;
       }
     }
     *aIncomingWhitespace = PR_FALSE;
   } else {
     PRBool inWhitespace = *aIncomingWhitespace;
     PRUint32 i;
@@ -192,31 +190,29 @@ nsTextFrameUtils::TransformText(const PR
                                 PRPackedBool* aIncomingWhitespace,
                                 gfxSkipCharsBuilder* aSkipChars,
                                 PRUint32* aAnalysisFlags)
 {
   PRUint32 flags = 0;
   PRUint8* outputStart = aOutput;
 
   if (!aCompressWhitespace) {
-    // Convert tabs to spaces and skip discardables.
+    // Skip discardables.
     PRUint32 i;
     for (i = 0; i < aLength; ++i) {
       PRUint8 ch = *aText++;
-      if (ch == '\t') {
-        flags |= TEXT_HAS_TAB|TEXT_WAS_TRANSFORMED;
-        aSkipChars->KeepChar();
-        *aOutput++ = ' ';
-      } else if (IsDiscardable(ch, &flags)) {
+      if (IsDiscardable(ch, &flags)) {
         aSkipChars->SkipChar();
       } else {
         aSkipChars->KeepChar();
         if (ch == CH_NBSP) {
           ch = ' ';
           flags |= TEXT_WAS_TRANSFORMED;
+        } else if (ch == '\t') {
+          flags |= TEXT_HAS_TAB;
         }
         *aOutput++ = ch;
       }
     }
     *aIncomingWhitespace = PR_FALSE;
   } else {
     PRBool inWhitespace = *aIncomingWhitespace;
     PRUint32 i;