bug 332636 - fix editor's handling of surrogate-pair combining marks with backspace. r=ehsan a=roc
authorJonathan Kew <jfkthame@gmail.com>
Thu, 16 Dec 2010 14:17:41 -0800
changeset 59604 b433f7b6d033be895ae1d279efd95f54bb9a9ea0
parent 59603 53dc4a2c24efe9d92ca1be228d83d5665a544669
child 59605 b84f528e2e103b6a22b52cab057c7787e0d1227c
push id1
push usershaver@mozilla.com
push dateTue, 04 Jan 2011 17:58:04 +0000
reviewersehsan, roc
bugs332636
milestone2.0b9pre
bug 332636 - fix editor's handling of surrogate-pair combining marks with backspace. r=ehsan a=roc
accessible/src/html/nsHyperTextAccessible.cpp
editor/libeditor/html/tests/test_bug332636.html
layout/generic/nsBRFrame.cpp
layout/generic/nsContainerFrame.cpp
layout/generic/nsContainerFrame.h
layout/generic/nsFrame.cpp
layout/generic/nsFrame.h
layout/generic/nsIFrame.h
layout/generic/nsInlineFrame.cpp
layout/generic/nsInlineFrame.h
layout/generic/nsSelection.cpp
layout/generic/nsTextFrame.h
layout/generic/nsTextFrameThebes.cpp
--- a/accessible/src/html/nsHyperTextAccessible.cpp
+++ b/accessible/src/html/nsHyperTextAccessible.cpp
@@ -902,17 +902,17 @@ nsresult nsHyperTextAccessible::GetTextH
   else if (aOffset < 0) {
     return NS_ERROR_FAILURE;
   }
 
   nsSelectionAmount amount;
   PRBool needsStart = PR_FALSE;
   switch (aBoundaryType) {
     case BOUNDARY_CHAR:
-      amount = eSelectCharacter;
+      amount = eSelectCluster;
       if (aType == eGetAt)
         aType = eGetAfter; // Avoid returning 2 characters
       break;
 
     case BOUNDARY_WORD_START:
       needsStart = PR_TRUE;
       amount = eSelectWord;
       break;
--- a/editor/libeditor/html/tests/test_bug332636.html
+++ b/editor/libeditor/html/tests/test_bug332636.html
@@ -9,18 +9,25 @@ https://bugzilla.mozilla.org/show_bug.cg
   <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
   <script type="text/javascript" src="/tests/SimpleTest/EventUtils.js"></script>  
 </head>
 <body>
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=332636">Mozilla Bug 332636</a>
 <p id="display"></p>
 <div id="content">
-  <div id="edit" contenteditable="true">a𐐀b</div>
-  <div id="edit2" contenteditable="true">a&#x10a0f;b</div>
+  <div id="edit0" contenteditable="true">axb</div><!-- reference: plane 0 base character -->
+  <div id="edit1" contenteditable="true">a&#x0308;b</div><!-- reference: plane 0 diacritic -->
+  <div id="edit2" contenteditable="true">a&#x10400;b</div><!-- plane 1 base character -->
+  <div id="edit3" contenteditable="true">a&#x10a0f;b</div><!-- plane 1 diacritic -->
+
+  <div id="edit0b" contenteditable="true">axb</div><!-- reference: plane 0 base character -->
+  <div id="edit1b" contenteditable="true">a&#x0308;b</div><!-- reference: plane 0 diacritic -->
+  <div id="edit2b" contenteditable="true">a&#x10400;b</div><!-- plane 1 base character -->
+  <div id="edit3b" contenteditable="true">a&#x10a0f;b</div><!-- plane 1 diacritic -->
 </div>
 <pre id="test">
 <script type="application/javascript">
 
 /** Test for Bug 332636 **/
 
 SimpleTest.waitForExplicitFinish();
 addLoadEvent(runTest);
@@ -28,18 +35,42 @@ addLoadEvent(runTest);
 function test(edit) {
   edit.focus();
   var sel = window.getSelection();
   sel.collapse(edit.childNodes[0], edit.textContent.length - 1);
   synthesizeKey("VK_BACK_SPACE", {});
   is(edit.textContent, "ab", "The backspace key should delete the UTF-16 surrogate pair correctly");
 }
 
+function testWithMove(edit, offset) {
+  edit.focus();
+  var sel = window.getSelection();
+  sel.collapse(edit.childNodes[0], 0);
+  var i;
+  for (i = 0; i < offset; ++i) {
+    synthesizeKey("VK_RIGHT", {});
+    synthesizeKey("VK_LEFT", {});
+    synthesizeKey("VK_RIGHT", {});
+  }
+  synthesizeKey("VK_BACK_SPACE", {});
+  is(edit.textContent, "ab", "The backspace key should delete the UTF-16 surrogate pair correctly");
+}
+
 function runTest() {
-  test(document.getElementById("edit"));
+  /* test backspace-deletion of the middle character */
+  test(document.getElementById("edit0"));
+  test(document.getElementById("edit1"));
   test(document.getElementById("edit2"));
+  test(document.getElementById("edit3"));
+
+  /* extra tests with the use of RIGHT and LEFT to get to the right place */
+  testWithMove(document.getElementById("edit0b"), 2);
+  testWithMove(document.getElementById("edit1b"), 1);
+  testWithMove(document.getElementById("edit2b"), 2);
+  testWithMove(document.getElementById("edit3b"), 1);
+
   SimpleTest.finish();
 }
 
 </script>
 </pre>
 </body>
 </html>
--- a/layout/generic/nsBRFrame.cpp
+++ b/layout/generic/nsBRFrame.cpp
@@ -64,17 +64,18 @@ class BRFrame : public nsFrame {
 public:
   NS_DECL_FRAMEARENA_HELPERS
 
   friend nsIFrame* NS_NewBRFrame(nsIPresShell* aPresShell, nsStyleContext* aContext);
 
   virtual ContentOffsets CalcContentOffsetsFromFramePoint(nsPoint aPoint);
 
   virtual PRBool PeekOffsetNoAmount(PRBool aForward, PRInt32* aOffset);
-  virtual PRBool PeekOffsetCharacter(PRBool aForward, PRInt32* aOffset);
+  virtual PRBool PeekOffsetCharacter(PRBool aForward, PRInt32* aOffset,
+                                     PRBool aRespectClusters = PR_TRUE);
   virtual PRBool PeekOffsetWord(PRBool aForward, PRBool aWordSelectEatSpace, PRBool aIsKeyboardSelect,
                                 PRInt32* aOffset, PeekWordState* aState);
 
   NS_IMETHOD Reflow(nsPresContext* aPresContext,
                     nsHTMLReflowMetrics& aDesiredSize,
                     const nsHTMLReflowState& aReflowState,
                     nsReflowStatus& aStatus);
   virtual void AddInlineMinWidth(nsIRenderingContext *aRenderingContext,
@@ -269,17 +270,18 @@ BRFrame::PeekOffsetNoAmount(PRBool aForw
     *aOffset = 0;
     return PR_TRUE;
   }
   // Otherwise, stop if we hit the beginning, continue (forward) if we hit the end.
   return (startOffset == 0);
 }
 
 PRBool
-BRFrame::PeekOffsetCharacter(PRBool aForward, PRInt32* aOffset)
+BRFrame::PeekOffsetCharacter(PRBool aForward, PRInt32* aOffset,
+                             PRBool aRespectClusters)
 {
   NS_ASSERTION (aOffset && *aOffset <= 1, "aOffset out of range");
   // Keep going. The actual line jumping will stop us.
   return PR_FALSE;
 }
 
 PRBool
 BRFrame::PeekOffsetWord(PRBool aForward, PRBool aWordSelectEatSpace, PRBool aIsKeyboardSelect,
--- a/layout/generic/nsContainerFrame.cpp
+++ b/layout/generic/nsContainerFrame.cpp
@@ -392,17 +392,18 @@ PRBool
 nsContainerFrame::PeekOffsetNoAmount(PRBool aForward, PRInt32* aOffset)
 {
   NS_ASSERTION (aOffset && *aOffset <= 1, "aOffset out of range");
   // Don't allow the caret to stay in an empty (leaf) container frame.
   return PR_FALSE;
 }
 
 PRBool
-nsContainerFrame::PeekOffsetCharacter(PRBool aForward, PRInt32* aOffset)
+nsContainerFrame::PeekOffsetCharacter(PRBool aForward, PRInt32* aOffset,
+                                      PRBool aRespectClusters)
 {
   NS_ASSERTION (aOffset && *aOffset <= 1, "aOffset out of range");
   // Don't allow the caret to stay in an empty (leaf) container frame.
   return PR_FALSE;
 }
 
 /////////////////////////////////////////////////////////////////////////////
 // Helper member functions
--- a/layout/generic/nsContainerFrame.h
+++ b/layout/generic/nsContainerFrame.h
@@ -93,17 +93,18 @@ public:
 
   virtual nsFrameList GetChildList(nsIAtom* aListName) const;
   virtual nsIAtom* GetAdditionalChildListName(PRInt32 aIndex) const;
   virtual void DestroyFrom(nsIFrame* aDestructRoot);
   virtual void ChildIsDirty(nsIFrame* aChild);
 
   virtual PRBool IsLeaf() const;
   virtual PRBool PeekOffsetNoAmount(PRBool aForward, PRInt32* aOffset);
-  virtual PRBool PeekOffsetCharacter(PRBool aForward, PRInt32* aOffset);
+  virtual PRBool PeekOffsetCharacter(PRBool aForward, PRInt32* aOffset,
+                                     PRBool aRespectClusters = PR_TRUE);
   
 #ifdef DEBUG
   NS_IMETHOD List(FILE* out, PRInt32 aIndent) const;
 #endif  
 
   // nsContainerFrame methods
 
   /**
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -5399,29 +5399,31 @@ nsIFrame::PeekOffset(nsPeekOffsetStruct*
 
   // Translate content offset to be relative to frame
   FrameContentRange range = GetRangeForFrame(this);
   PRInt32 offset = aPos->mStartOffset - range.start;
   nsIFrame* current = this;
   
   switch (aPos->mAmount) {
     case eSelectCharacter:
+    case eSelectCluster:
     {
       PRBool eatingNonRenderableWS = PR_FALSE;
       PRBool done = PR_FALSE;
       PRBool jumpedLine = PR_FALSE;
       
       while (!done) {
         PRBool movingInFrameDirection =
           IsMovingInFrameDirection(current, aPos->mDirection, aPos->mVisual);
 
         if (eatingNonRenderableWS)
           done = current->PeekOffsetNoAmount(movingInFrameDirection, &offset); 
         else
-          done = current->PeekOffsetCharacter(movingInFrameDirection, &offset); 
+          done = current->PeekOffsetCharacter(movingInFrameDirection, &offset,
+                                              aPos->mAmount == eSelectCluster);
 
         if (!done) {
           result =
             current->GetFrameFromDirection(aPos->mDirection, aPos->mVisual,
                                            aPos->mJumpLines, aPos->mScrollViewStop,
                                            &current, &offset, &jumpedLine);
           if (NS_FAILED(result))
             return result;
@@ -5700,17 +5702,18 @@ PRBool
 nsFrame::PeekOffsetNoAmount(PRBool aForward, PRInt32* aOffset)
 {
   NS_ASSERTION (aOffset && *aOffset <= 1, "aOffset out of range");
   // Sure, we can stop right here.
   return PR_TRUE;
 }
 
 PRBool
-nsFrame::PeekOffsetCharacter(PRBool aForward, PRInt32* aOffset)
+nsFrame::PeekOffsetCharacter(PRBool aForward, PRInt32* aOffset,
+                             PRBool aRespectClusters)
 {
   NS_ASSERTION (aOffset && *aOffset <= 1, "aOffset out of range");
   PRInt32 startOffset = *aOffset;
   // A negative offset means "end of frame", which in our case means offset 1.
   if (startOffset < 0)
     startOffset = 1;
   if (aForward == (startOffset == 0)) {
     // We're before the frame and moving forward, or after it and moving backwards:
--- a/layout/generic/nsFrame.h
+++ b/layout/generic/nsFrame.h
@@ -233,17 +233,18 @@ public:
   virtual PRBool IsContainingBlock() const;
 
   NS_IMETHOD  GetSelected(PRBool *aSelected) const;
   NS_IMETHOD  IsSelectable(PRBool* aIsSelectable, PRUint8* aSelectStyle) const;
 
   NS_IMETHOD  GetSelectionController(nsPresContext *aPresContext, nsISelectionController **aSelCon);
 
   virtual PRBool PeekOffsetNoAmount(PRBool aForward, PRInt32* aOffset);
-  virtual PRBool PeekOffsetCharacter(PRBool aForward, PRInt32* aOffset);
+  virtual PRBool PeekOffsetCharacter(PRBool aForward, PRInt32* aOffset,
+                                     PRBool aRespectClusters = PR_TRUE);
   virtual PRBool PeekOffsetWord(PRBool aForward, PRBool aWordSelectEatSpace, PRBool aIsKeyboardSelect,
                                 PRInt32* aOffset, PeekWordState *aState);
   /**
    * Check whether we should break at a boundary between punctuation and
    * non-punctuation. Only call it at a punctuation boundary
    * (i.e. exactly one of the previous and next characters are punctuation).
    * @param aForward true if we're moving forward in content order
    * @param aPunctAfter true if the next character is punctuation
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -284,23 +284,28 @@ typedef PRUint64 nsFrameState;
 // Helper macros
 #define NS_SUBTREE_DIRTY(_frame)  \
   (((_frame)->GetStateBits() &      \
     (NS_FRAME_IS_DIRTY | NS_FRAME_HAS_DIRTY_CHILDREN)) != 0)
 
 //----------------------------------------------------------------------
 
 enum nsSelectionAmount {
-  eSelectCharacter = 0,
-  eSelectWord      = 1,
-  eSelectLine      = 2,  //previous drawn line in flow.
-  eSelectBeginLine = 3,
-  eSelectEndLine   = 4,
-  eSelectNoAmount  = 5,   //just bounce back current offset.
-  eSelectParagraph = 6    //select a "paragraph"
+  eSelectCharacter = 0, // a single Unicode character;
+                        // do not use this (prefer Cluster) unless you
+                        // are really sure it's what you want
+  eSelectCluster   = 1, // a grapheme cluster: this is usually the right
+                        // choice for movement or selection by "character"
+                        // as perceived by the user
+  eSelectWord      = 2,
+  eSelectLine      = 3, // previous drawn line in flow.
+  eSelectBeginLine = 4,
+  eSelectEndLine   = 5,
+  eSelectNoAmount  = 6, // just bounce back current offset.
+  eSelectParagraph = 7  // select a "paragraph"
 };
 
 enum nsDirection {
   eDirNext    = 0,
   eDirPrevious= 1
 };
 
 enum nsSpread {
@@ -2675,21 +2680,25 @@ protected:
    */
   virtual PRBool PeekOffsetNoAmount(PRBool aForward, PRInt32* aOffset) = 0;
   
   /**
    * Search the frame for the next character
    * @param  aForward [in] Are we moving forward (or backward) in content order.
    * @param  aOffset [in/out] At what offset into the frame to start looking.
    *         on output - what offset was reached (whether or not we found a place to stop).
+   * @param  aRespectClusters [in] Whether to restrict result to valid cursor locations
+   *         (between grapheme clusters) - default TRUE maintains "normal" behavior,
+   *         FALSE is used for selection by "code unit" (instead of "character")
    * @return PR_TRUE: An appropriate offset was found within this frame,
    *         and is given by aOffset.
    *         PR_FALSE: Not found within this frame, need to try the next frame.
    */
-  virtual PRBool PeekOffsetCharacter(PRBool aForward, PRInt32* aOffset) = 0;
+  virtual PRBool PeekOffsetCharacter(PRBool aForward, PRInt32* aOffset,
+                                     PRBool aRespectClusters = PR_TRUE) = 0;
   
   /**
    * Search the frame for the next word boundary
    * @param  aForward [in] Are we moving forward (or backward) in content order.
    * @param  aWordSelectEatSpace [in] PR_TRUE: look for non-whitespace following
    *         whitespace (in the direction of movement).
    *         PR_FALSE: look for whitespace following non-whitespace (in the
    *         direction  of movement).
--- a/layout/generic/nsInlineFrame.cpp
+++ b/layout/generic/nsInlineFrame.cpp
@@ -161,17 +161,18 @@ nsInlineFrame::IsEmpty()
     if (!kid->IsEmpty())
       return PR_FALSE;
   }
 
   return PR_TRUE;
 }
 
 PRBool
-nsInlineFrame::PeekOffsetCharacter(PRBool aForward, PRInt32* aOffset)
+nsInlineFrame::PeekOffsetCharacter(PRBool aForward, PRInt32* aOffset,
+                                   PRBool aRespectClusters)
 {
   // Override the implementation in nsFrame, to skip empty inline frames
   NS_ASSERTION (aOffset && *aOffset <= 1, "aOffset out of range");
   PRInt32 startOffset = *aOffset;
   if (startOffset < 0)
     startOffset = 1;
   if (aForward == (startOffset == 0)) {
     // We're before the frame and moving forward, or after it and moving backwards:
--- a/layout/generic/nsInlineFrame.h
+++ b/layout/generic/nsInlineFrame.h
@@ -92,17 +92,18 @@ public:
   {
     return nsInlineFrameSuper::IsFrameOfType(aFlags &
       ~(nsIFrame::eBidiInlineContainer | nsIFrame::eLineParticipant));
   }
 
   virtual PRBool IsEmpty();
   virtual PRBool IsSelfEmpty();
 
-  virtual PRBool PeekOffsetCharacter(PRBool aForward, PRInt32* aOffset);
+  virtual PRBool PeekOffsetCharacter(PRBool aForward, PRInt32* aOffset,
+                                     PRBool aRespectClusters = PR_TRUE);
   
   // nsIHTMLReflow overrides
   virtual void AddInlineMinWidth(nsIRenderingContext *aRenderingContext,
                                  InlineMinWidthData *aData);
   virtual void AddInlinePrefWidth(nsIRenderingContext *aRenderingContext,
                                   InlinePrefWidthData *aData);
   virtual nsSize ComputeSize(nsIRenderingContext *aRenderingContext,
                              nsSize aCBSize, nscoord aAvailableWidth,
--- a/layout/generic/nsSelection.cpp
+++ b/layout/generic/nsSelection.cpp
@@ -1242,17 +1242,17 @@ nsFrameSelection::MoveCaret(PRUint32    
   default :return NS_ERROR_FAILURE;
   }
   PostReason(nsISelectionListener::KEYPRESS_REASON);
   if (NS_SUCCEEDED(result = frame->PeekOffset(&pos)) && pos.mResultContent)
   {
     nsIFrame *theFrame;
     PRInt32 currentOffset, frameStart, frameEnd;
 
-    if (aAmount == eSelectCharacter || aAmount == eSelectWord)
+    if (aAmount >= eSelectCharacter && aAmount <= eSelectWord)
     {
       // For left/right, PeekOffset() sets pos.mResultFrame correctly, but does not set pos.mAttachForward,
       // so determine the hint here based on the result frame and offset:
       // If we're at the end of a text frame, set the hint to HINTLEFT to indicate that we
       // want the caret displayed at the end of this frame, not at the beginning of the next one.
       theFrame = pos.mResultFrame;
       theFrame->GetOffsets(frameStart, frameEnd);
       currentOffset = pos.mContentOffset;
@@ -2186,25 +2186,25 @@ nsFrameSelection::CommonPageMove(PRBool 
   HandleClick(offsets.content, offsets.offset,
               offsets.offset, aExtend, PR_FALSE, PR_TRUE);
 }
 
 nsresult
 nsFrameSelection::CharacterMove(PRBool aForward, PRBool aExtend)
 {
   if (aForward)
-    return MoveCaret(nsIDOMKeyEvent::DOM_VK_RIGHT,aExtend,eSelectCharacter);
+    return MoveCaret(nsIDOMKeyEvent::DOM_VK_RIGHT, aExtend, eSelectCluster);
   else
-    return MoveCaret(nsIDOMKeyEvent::DOM_VK_LEFT,aExtend,eSelectCharacter);
+    return MoveCaret(nsIDOMKeyEvent::DOM_VK_LEFT, aExtend, eSelectCluster);
 }
 
 nsresult
 nsFrameSelection::CharacterExtendForDelete()
 {
-  return MoveCaret(nsIDOMKeyEvent::DOM_VK_DELETE, PR_TRUE, eSelectCharacter);
+  return MoveCaret(nsIDOMKeyEvent::DOM_VK_DELETE, PR_TRUE, eSelectCluster);
 }
 
 nsresult
 nsFrameSelection::CharacterExtendForBackspace()
 {
   return MoveCaret(nsIDOMKeyEvent::DOM_VK_BACK_SPACE, PR_TRUE, eSelectCharacter);
 }
 
@@ -5842,17 +5842,17 @@ nsTypedSelection::Modify(const nsAString
                    aDirection.LowerCaseEqualsLiteral("right");
 
   PRBool extend  = aAlter.LowerCaseEqualsLiteral("extend");
 
   // The PRUint32 casts below prevent an enum mismatch warning.
   nsSelectionAmount amount;
   PRUint32 keycode;
   if (aGranularity.LowerCaseEqualsLiteral("character")) {
-    amount = eSelectCharacter;
+    amount = eSelectCluster;
     keycode = forward ? (PRUint32) nsIDOMKeyEvent::DOM_VK_RIGHT :
                         (PRUint32) nsIDOMKeyEvent::DOM_VK_LEFT;
   }
   else if (aGranularity.LowerCaseEqualsLiteral("word")) {
     amount = eSelectWord;
     keycode = forward ? (PRUint32) nsIDOMKeyEvent::DOM_VK_RIGHT :
                         (PRUint32) nsIDOMKeyEvent::DOM_VK_LEFT;
   }
--- a/layout/generic/nsTextFrame.h
+++ b/layout/generic/nsTextFrame.h
@@ -164,17 +164,18 @@ public:
   virtual void SetSelected(PRBool        aSelected,
                            SelectionType aType);
   void SetSelectedRange(PRUint32 aStart,
                         PRUint32 aEnd,
                         PRBool aSelected,
                         SelectionType aType);
 
   virtual PRBool PeekOffsetNoAmount(PRBool aForward, PRInt32* aOffset);
-  virtual PRBool PeekOffsetCharacter(PRBool aForward, PRInt32* aOffset);
+  virtual PRBool PeekOffsetCharacter(PRBool aForward, PRInt32* aOffset,
+                                     PRBool aRespectClusters = PR_TRUE);
   virtual PRBool PeekOffsetWord(PRBool aForward, PRBool aWordSelectEatSpace, PRBool aIsKeyboardSelect,
                                 PRInt32* aOffset, PeekWordState* aState);
 
   NS_IMETHOD CheckVisibility(nsPresContext* aContext, PRInt32 aStartIndex, PRInt32 aEndIndex, PRBool aRecurse, PRBool *aFinished, PRBool *_retval);
   
   // Update offsets to account for new length. This may clear mTextRun.
   void SetLength(PRInt32 aLength, nsLineLayout* aLineLayout);
   
--- a/layout/generic/nsTextFrameThebes.cpp
+++ b/layout/generic/nsTextFrameThebes.cpp
@@ -5575,29 +5575,45 @@ private:
   PRInt32                     mDirection;
   PRInt32                     mCharIndex;
   nsTextFrame::TrimmedOffsets mTrimmed;
   nsTArray<PRPackedBool>      mWordBreaks;
   PRPackedBool                mHaveWordBreak;
 };
 
 static PRBool
-IsAcceptableCaretPosition(const gfxSkipCharsIterator& aIter, gfxTextRun* aTextRun,
+IsAcceptableCaretPosition(const gfxSkipCharsIterator& aIter,
+                          PRBool aRespectClusters,
+                          gfxTextRun* aTextRun,
                           nsIFrame* aFrame)
 {
   if (aIter.IsOriginalCharSkipped())
     return PR_FALSE;
   PRUint32 index = aIter.GetSkippedOffset();
-  if (!aTextRun->IsClusterStart(index))
+  if (aRespectClusters && !aTextRun->IsClusterStart(index))
     return PR_FALSE;
+  if (index > 0) {
+    // Check whether the proposed position is in between the two halves of a
+    // surrogate pair; if so, this is not a valid character boundary.
+    // (In the case where we are respecting clusters, we won't actually get
+    // this far because the low surrogate is also marked as non-clusterStart
+    // so we'll return FALSE above.)
+    // If the textrun is 8-bit it can't have any surrogates, so we only need
+    // to check the actual characters if GetTextUnicode() returns non-null.
+    const PRUnichar *txt = aTextRun->GetTextUnicode();
+    if (txt && NS_IS_LOW_SURROGATE(txt[index]) &&
+               NS_IS_HIGH_SURROGATE(txt[index-1]))
+      return PR_FALSE;
+  }
   return PR_TRUE;
 }
 
 PRBool
-nsTextFrame::PeekOffsetCharacter(PRBool aForward, PRInt32* aOffset)
+nsTextFrame::PeekOffsetCharacter(PRBool aForward, PRInt32* aOffset,
+                                 PRBool aRespectClusters)
 {
   PRInt32 contentLength = GetContentLength();
   NS_ASSERTION(aOffset && *aOffset <= contentLength, "aOffset out of range");
 
   PRBool selectable;
   PRUint8 selectStyle;  
   IsSelectable(&selectable, &selectStyle);
   if (selectStyle == NS_STYLE_USER_SELECT_ALL)
@@ -5612,34 +5628,34 @@ nsTextFrame::PeekOffsetCharacter(PRBool 
   // A negative offset means "end of frame".
   PRInt32 startOffset = GetContentOffset() + (*aOffset < 0 ? contentLength : *aOffset);
 
   if (!aForward) {
     // If at the beginning of the line, look at the previous continuation
     for (PRInt32 i = NS_MIN(trimmed.GetEnd(), startOffset) - 1;
          i >= trimmed.mStart; --i) {
       iter.SetOriginalOffset(i);
-      if (IsAcceptableCaretPosition(iter, mTextRun, this)) {
+      if (IsAcceptableCaretPosition(iter, aRespectClusters, mTextRun, this)) {
         *aOffset = i - mContentOffset;
         return PR_TRUE;
       }
     }
     *aOffset = 0;
   } else {
     // If we're at the end of a line, look at the next continuation
     iter.SetOriginalOffset(startOffset);
     if (startOffset <= trimmed.GetEnd() &&
         !(startOffset < trimmed.GetEnd() &&
           GetStyleText()->NewlineIsSignificant() &&
           iter.GetSkippedOffset() < mTextRun->GetLength() &&
           mTextRun->GetChar(iter.GetSkippedOffset()) == '\n')) {
       for (PRInt32 i = startOffset + 1; i <= trimmed.GetEnd(); ++i) {
         iter.SetOriginalOffset(i);
         if (i == trimmed.GetEnd() ||
-            IsAcceptableCaretPosition(iter, mTextRun, this)) {
+            IsAcceptableCaretPosition(iter, aRespectClusters, mTextRun, this)) {
           *aOffset = i - mContentOffset;
           return PR_TRUE;
         }
       }
     }
     *aOffset = contentLength;
   }