Bug 1216096: restore previous RTL caret behaviour by backout of bug 1164963, bug 1177505, and bug 1180417. r=jfkthame, a=rkothari
authorSimon Montagu <smontagu@smontagu.org>
Tue, 01 Dec 2015 23:52:41 -0800
changeset 296854 274a69572dda
parent 296851 78586d0f2f3c
child 296855 47075790a159
push id5372
push usersmontagu@smontagu.org
push dateWed, 02 Dec 2015 08:41:02 +0000
treeherdermozilla-beta@274a69572dda [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjfkthame, rkothari
bugs1216096, 1164963, 1177505, 1180417
milestone43.0
Bug 1216096: restore previous RTL caret behaviour by backout of bug 1164963, bug 1177505, and bug 1180417. r=jfkthame, a=rkothari
layout/base/nsBidiPresUtils.cpp
layout/base/nsCaret.cpp
layout/base/nsCaret.h
layout/base/tests/bug646382-1-ref.html
layout/base/tests/bug646382-2-ref.html
layout/generic/nsFrameSelection.h
layout/generic/nsSelection.cpp
layout/generic/nsTextFrame.cpp
--- a/layout/base/nsBidiPresUtils.cpp
+++ b/layout/base/nsBidiPresUtils.cpp
@@ -729,52 +729,41 @@ nsBidiPresUtils::ResolveParagraph(nsBloc
          (void*)aBlockFrame, NS_ConvertUTF16toUTF8(aBpd->mBuffer).get(), frameCount, runCount);
 #ifdef REALLY_NOISY_BIDI
   printf(" block frame tree=:\n");
   aBlockFrame->List(stdout, 0);
 #endif
 #endif
 #endif
 
-  nsIFrame* frame0 = frameCount > 0 ? aBpd->FrameAt(0) : nullptr;
-
-  // Non-bidi frames
-  if (runCount == 1 &&
+  if (runCount == 1 && frameCount == 1 &&
       aBpd->mParagraphDepth == 0 && aBpd->GetDirection() == NSBIDI_LTR &&
-      aBpd->GetParaLevel() == 0 &&
-      frame0 && frame0 != NS_BIDI_CONTROL_FRAME &&
-      !frame0->Properties().Get(nsIFrame::EmbeddingLevelProperty()) &&
-      !frame0->Properties().Get(nsIFrame::BaseLevelProperty())) {
-    // We have a left-to-right frame in a left-to-right paragraph,
+      aBpd->GetParaLevel() == 0) {
+    // We have a single left-to-right frame in a left-to-right paragraph,
     // without bidi isolation from the surrounding text.
-    // The embedding level and base level frame properties aren't
+    // Make sure that the embedding level and base level frame properties aren't
     // set (because if they are this frame used to have some other direction,
-    // so we can't do this optimization)
-
-    // Make all continuations fluid within this run
-    for (int i = 0; i < frameCount; ++i) {
-      nsIFrame* frame = aBpd->FrameAt(i);
-      if (frame && frame != NS_BIDI_CONTROL_FRAME) {
-        JoinInlineAncestors(frame);
-      }
-    }
-
+    // so we can't do this optimization), and we're done.
+    nsIFrame* frame = aBpd->FrameAt(0);
+    if (frame != NS_BIDI_CONTROL_FRAME &&
+        !frame->Properties().Get(nsIFrame::EmbeddingLevelProperty()) &&
+        !frame->Properties().Get(nsIFrame::BaseLevelProperty())) {
 #ifdef DEBUG
 #ifdef NOISY_BIDI
-    printf("early return for single direction frame %p\n", (void*)frame);
+      printf("early return for single direction frame %p\n", (void*)frame);
 #endif
 #endif
-
-    return NS_OK;
+      frame->AddStateBits(NS_FRAME_IS_BIDI);
+      return NS_OK;
+    }
   }
 
   nsIFrame* firstFrame = nullptr;
   nsIFrame* lastFrame = nullptr;
 
-  // Bidi frames
   for (; ;) {
     if (fragmentLength <= 0) {
       // Get the next frame from mLogicalFrames
       if (++frameIndex >= frameCount) {
         break;
       }
       frame = aBpd->FrameAt(frameIndex);
       if (frame == NS_BIDI_CONTROL_FRAME ||
--- a/layout/base/nsCaret.cpp
+++ b/layout/base/nsCaret.cpp
@@ -110,24 +110,19 @@ AdjustCaretFrameForLineEnd(nsIFrame** aF
       NS_ASSERTION(r->GetType() == nsGkAtoms::textFrame, "Expected text frame");
       *aOffset = (static_cast<nsTextFrame*>(r))->GetContentEnd();
       return;
     }
   }
 }
 
 static bool
-IsKeyboardRTL()
+IsBidiUI()
 {
-  bool isKeyboardRTL = false;
-  nsIBidiKeyboard* bidiKeyboard = nsContentUtils::GetBidiKeyboard();
-  if (bidiKeyboard) {
-    bidiKeyboard->IsLangRTL(&isKeyboardRTL);
-  }
-  return isKeyboardRTL;
+  return Preferences::GetBool("bidi.browser.ui");
 }
 
 nsCaret::nsCaret()
 : mOverrideOffset(0)
 , mBlinkCount(-1)
 , mHideCount(0)
 , mIsBlinkOn(false)
 , mVisible(false)
@@ -498,40 +493,28 @@ nsCaret::SetCaretPosition(nsIDOMNode* aN
 {
   mOverrideContent = do_QueryInterface(aNode);
   mOverrideOffset = aOffset;
 
   ResetBlinking();
   SchedulePaint();
 }
 
-bool
-nsCaret::IsBidiUI()
-{
-  nsIFrame* frame = nullptr;
-
-  if(Selection* selection = GetSelectionInternal()) {
-    int32_t contentOffset;
-    frame = GetFrameAndOffset(selection, mOverrideContent, mOverrideOffset,
-                              &contentOffset);
-  }
-
-  return (frame && frame->GetStateBits() & NS_FRAME_IS_BIDI) ||
-         Preferences::GetBool("bidi.browser.ui");
-}
-
 void
 nsCaret::CheckSelectionLanguageChange()
 {
   if (!IsBidiUI()) {
     return;
   }
 
-  bool isKeyboardRTL = IsKeyboardRTL();
-
+  bool isKeyboardRTL = false;
+  nsIBidiKeyboard* bidiKeyboard = nsContentUtils::GetBidiKeyboard();
+  if (bidiKeyboard) {
+    bidiKeyboard->IsLangRTL(&isKeyboardRTL);
+  }
   // Call SelectionLanguageChange on every paint. Mostly it will be a noop
   // but it should be fast anyway. This guarantees we never paint the caret
   // at the wrong place.
   Selection* selection = GetSelectionInternal();
   if (selection) {
     selection->SelectionLanguageChange(isKeyboardRTL);
   }
 }
@@ -710,19 +693,18 @@ nsCaret::GetCaretFrameForNodeOffset(nsFr
   //
   // Direction Style from visibility->mDirection
   // ------------------
   // NS_STYLE_DIRECTION_LTR : LTR or Default
   // NS_STYLE_DIRECTION_RTL
   if (theFrame->PresContext()->BidiEnabled())
   {
     // If there has been a reflow, take the caret Bidi level to be the level of the current frame
-    if (aBidiLevel & BIDI_LEVEL_UNDEFINED) {
+    if (aBidiLevel & BIDI_LEVEL_UNDEFINED)
       aBidiLevel = NS_GET_EMBEDDING_LEVEL(theFrame);
-    }
 
     int32_t start;
     int32_t end;
     nsIFrame* frameBefore;
     nsIFrame* frameAfter;
     nsBidiLevel levelBefore; // Bidi level of the character before the caret
     nsBidiLevel levelAfter;  // Bidi level of the character after the caret
 
@@ -935,32 +917,32 @@ nsCaret::ComputeCaretRects(nsIFrame* aFr
   if (NS_STYLE_DIRECTION_RTL == vis->mDirection) {
     if (isVertical) {
       aCaretRect->y -= aCaretRect->height;
     } else {
       aCaretRect->x -= aCaretRect->width;
     }
   }
 
+  // Simon -- make a hook to draw to the left or right of the caret to show keyboard language direction
   aHookRect->SetEmpty();
 
-  Selection* selection = GetSelectionInternal();
-  if (!selection || !selection->GetFrameSelection()) {
+  if (!IsBidiUI()) {
     return;
   }
 
-  if (IsBidiUI() || IsKeyboardRTL()) {
-    // If caret level is RTL, draw the hook on the left; if LTR, to the right
+  bool isCaretRTL;
+  nsIBidiKeyboard* bidiKeyboard = nsContentUtils::GetBidiKeyboard();
+  // if bidiKeyboard->IsLangRTL() fails, there is no way to tell the
+  // keyboard direction, or the user has no right-to-left keyboard
+  // installed, so we never draw the hook.
+  if (bidiKeyboard && NS_SUCCEEDED(bidiKeyboard->IsLangRTL(&isCaretRTL))) {
+    // If keyboard language is RTL, draw the hook on the left; if LTR, to the right
     // The height of the hook rectangle is the same as the width of the caret
     // rectangle.
-    int caretBidiLevel = selection->GetFrameSelection()->GetCaretBidiLevel();
-    if (caretBidiLevel & BIDI_LEVEL_UNDEFINED) {
-      caretBidiLevel = NS_GET_EMBEDDING_LEVEL(aFrame);
-    }
-    bool isCaretRTL = caretBidiLevel % 2;
     if (isVertical) {
       aHookRect->SetRect(aCaretRect->XMost() - bidiIndicatorSize,
                          aCaretRect->y + (isCaretRTL ? bidiIndicatorSize * -1 :
                                                        aCaretRect->height),
                          aCaretRect->height,
                          bidiIndicatorSize);
     } else {
       aHookRect->SetRect(aCaretRect->x + (isCaretRTL ? bidiIndicatorSize * -1 :
--- a/layout/base/nsCaret.h
+++ b/layout/base/nsCaret.h
@@ -178,17 +178,16 @@ class nsCaret final : public nsISelectio
                                        int32_t aOverrideOffset,
                                        int32_t* aFrameOffset);
 
     size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
 
 protected:
     static void   CaretBlinkCallback(nsITimer *aTimer, void *aClosure);
 
-    bool          IsBidiUI();
     void          CheckSelectionLanguageChange();
 
     void          ResetBlinking();
     void          StopBlinking();
 
     mozilla::dom::Selection* GetSelectionInternal();
 
     struct Metrics {
--- a/layout/base/tests/bug646382-1-ref.html
+++ b/layout/base/tests/bug646382-1-ref.html
@@ -1,18 +1,17 @@
 <html class="reftest-wait">
   <head>
     <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
   </head>
   <body onload="start()">
-    <textarea onfocus="done()" style="-moz-appearance: none">س&lrm;</textarea>
+    <textarea onfocus="done()" style="-moz-appearance: none">س</textarea>
     <script>
       var textarea = document.querySelector("textarea");
       function start() {
         textarea.focus();
       }
       function done() {
-        textarea.selectionStart = textarea.selectionEnd = 2;
         document.documentElement.removeAttribute("class");
       }
     </script>
   </body>
 </html>
--- a/layout/base/tests/bug646382-2-ref.html
+++ b/layout/base/tests/bug646382-2-ref.html
@@ -1,15 +1,14 @@
 <html class="reftest-wait">
   <body onload="start()">
-    <textarea dir="rtl" onfocus="done()" style="-moz-appearance: none">s&rlm;</textarea>
+    <textarea dir="rtl" onfocus="done()" style="-moz-appearance: none">s</textarea>
     <script>
       var textarea = document.querySelector("textarea");
       function start() {
         textarea.focus();
       }
       function done() {
-        textarea.selectionStart = textarea.selectionEnd = 2;
         document.documentElement.removeAttribute("class");
       }
     </script>
   </body>
 </html>
--- a/layout/generic/nsFrameSelection.h
+++ b/layout/generic/nsFrameSelection.h
@@ -616,19 +616,17 @@ private:
                      bool aContinueSelection,
                      bool aMultipleSelection);
 
   void BidiLevelFromMove(nsIPresShell* aPresShell,
                          nsIContent *aNode,
                          uint32_t aContentOffset,
                          nsSelectionAmount aAmount,
                          CaretAssociateHint aHint);
-  void BidiLevelFromClick(nsIContent *aNewFocus,
-                          uint32_t aContentOffset,
-                          CaretAssociateHint aHint);
+  void BidiLevelFromClick(nsIContent *aNewFocus, uint32_t aContentOffset);
   nsPrevNextBidiLevels GetPrevNextBidiLevels(nsIContent *aNode,
                                              uint32_t aContentOffset,
                                              CaretAssociateHint aHint,
                                              bool aJumpLines) const;
 
   bool AdjustForMaintainedSelection(nsIContent *aContent, int32_t aOffset);
 
 // post and pop reasons for notifications. we may stack these later
--- a/layout/generic/nsSelection.cpp
+++ b/layout/generic/nsSelection.cpp
@@ -1455,23 +1455,22 @@ void nsFrameSelection::BidiLevelFromMove
 
 /**
  * BidiLevelFromClick is called when the caret is repositioned by clicking the mouse
  *
  * @param aNode is the content node
  * @param aContentOffset is the new caret position, as an offset into aNode
  */
 void nsFrameSelection::BidiLevelFromClick(nsIContent *aNode,
-                                          uint32_t    aContentOffset,
-                                          CaretAssociateHint aHint)
+                                          uint32_t    aContentOffset)
 {
   nsIFrame* clickInFrame=nullptr;
   int32_t OffsetNotUsed;
 
-  clickInFrame = GetFrameForNodeOffset(aNode, aContentOffset, aHint, &OffsetNotUsed);
+  clickInFrame = GetFrameForNodeOffset(aNode, aContentOffset, mHint, &OffsetNotUsed);
   if (!clickInFrame)
     return;
 
   SetCaretBidiLevel(NS_GET_EMBEDDING_LEVEL(clickInFrame));
 }
 
 
 bool
@@ -1540,17 +1539,17 @@ nsFrameSelection::HandleClick(nsIContent
     if (!IsValidSelectionPoint(this, aNewFocus)) {
       mAncestorLimiter = nullptr;
     }
   }
 
   // Don't take focus when dragging off of a table
   if (!mDragSelectingCells)
   {
-    BidiLevelFromClick(aNewFocus, aContentOffset, aHint);
+    BidiLevelFromClick(aNewFocus, aContentOffset);
     PostReason(nsISelectionListener::MOUSEDOWN_REASON + nsISelectionListener::DRAG_REASON);
     if (aContinueSelection &&
         AdjustForMaintainedSelection(aNewFocus, aContentOffset))
       return NS_OK; //shift clicked to maintained selection. rejected.
 
     int8_t index = GetIndexFromSelectionType(nsISelectionController::SELECTION_NORMAL);
     AutoPrepareFocusRange prep(mDomSelections[index], aContinueSelection, aMultipleSelection);
     return TakeFocus(aNewFocus, aContentOffset, aContentEndOffset, aHint,
--- a/layout/generic/nsTextFrame.cpp
+++ b/layout/generic/nsTextFrame.cpp
@@ -621,29 +621,29 @@ int32_t nsTextFrame::GetContentEnd() con
 struct FlowLengthProperty {
   int32_t mStartOffset;
   // The offset of the next fixed continuation after mStartOffset, or
   // of the end of the text if there is none
   int32_t mEndFlowOffset;
 };
 
 int32_t nsTextFrame::GetInFlowContentLength() {
-  if (!PresContext()->BidiEnabled()) {
+  if (!(mState & NS_FRAME_IS_BIDI)) {
     return mContent->TextLength() - mContentOffset;
   }
 
   FlowLengthProperty* flowLength =
     static_cast<FlowLengthProperty*>(mContent->GetProperty(nsGkAtoms::flowlength));
 
   /**
    * This frame must start inside the cached flow. If the flow starts at
    * mContentOffset but this frame is empty, logically it might be before the
    * start of the cached flow.
    */
-  if (flowLength &&
+  if (flowLength && 
       (flowLength->mStartOffset < mContentOffset ||
        (flowLength->mStartOffset == mContentOffset && GetContentEnd() > mContentOffset)) &&
       flowLength->mEndFlowOffset > mContentOffset) {
 #ifdef DEBUG
     NS_ASSERTION(flowLength->mEndFlowOffset >= GetContentEnd(),
 		 "frame crosses fixed continuation boundary");
 #endif
     return flowLength->mEndFlowOffset - mContentOffset;