Bug 1100071 patch 3: don't pass around bidi levels when we only need the direction, r=dholbert
authorSimon Montagu <smontagu@smontagu.org>
Thu, 20 Nov 2014 12:45:23 +0200
changeset 216630 1bee60dc14ec7a2563de9a8c9e412143948e5cdf
parent 216629 53fb431ea9433f4c31517760502114dcea6fbc23
child 216631 68dd715f6f79a4fc540b7fffeec047cfe4dc3b6a
push idunknown
push userunknown
push dateunknown
reviewersdholbert
bugs1100071
milestone36.0a1
Bug 1100071 patch 3: don't pass around bidi levels when we only need the direction, r=dholbert
layout/base/nsBidiPresUtils.h
layout/generic/nsFrame.cpp
layout/generic/nsFrameList.cpp
layout/generic/nsILineIterator.h
layout/generic/nsSelection.cpp
--- a/layout/base/nsBidiPresUtils.h
+++ b/layout/base/nsBidiPresUtils.h
@@ -281,16 +281,40 @@ public:
    */
   static uint8_t GetParagraphDepth(nsIFrame* aFrame);
 
   /**
    * Get the bidi base level of the given (inline) frame.
    */
   static nsBidiLevel GetFrameBaseLevel(nsIFrame* aFrame);
 
+  /**
+   * Get an nsBidiDirection representing the direction implied by the
+   * bidi base level of the frame.
+   * @return NSBIDI_LTR (left-to-right) or NSBIDI_RTL (right-to-left)
+   *  NSBIDI_MIXED will never be returned.
+   */
+  static nsBidiDirection ParagraphDirection(nsIFrame* aFrame) {
+    return DIRECTION_FROM_LEVEL(GetFrameBaseLevel(aFrame));
+  }
+
+  /**
+   * Get an nsBidiDirection representing the direction implied by the
+   * bidi embedding level of the frame.
+   * @return NSBIDI_LTR (left-to-right) or NSBIDI_RTL (right-to-left)
+   *  NSBIDI_MIXED will never be returned.
+   */
+  static nsBidiDirection FrameDirection(nsIFrame* aFrame) {
+    return DIRECTION_FROM_LEVEL(GetFrameEmbeddingLevel(aFrame));
+  }
+
+  static bool IsFrameInParagraphDirection(nsIFrame* aFrame) {
+    return ParagraphDirection(aFrame) == FrameDirection(aFrame);
+  }
+
   enum Mode { MODE_DRAW, MODE_MEASURE };
 
   /**
    * Reorder plain text using the Unicode Bidi algorithm and send it to
    * a processor for rendering or measuring
    *
    * @param[in] aText  the string to be processed (in logical order)
    * @param aLength the number of characters in the string
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -6648,21 +6648,24 @@ nsIFrame::PeekOffset(nsPeekOffsetStruct*
 
       if (aPos->mVisual && PresContext()->BidiEnabled()) {
         bool lineIsRTL = it->GetDirection();
         bool isReordered;
         nsIFrame *lastFrame;
         result = it->CheckLineOrder(thisLine, &isReordered, &firstFrame, &lastFrame);
         baseFrame = endOfLine ? lastFrame : firstFrame;
         if (baseFrame) {
-          nsBidiLevel embeddingLevel = nsBidiPresUtils::GetFrameEmbeddingLevel(baseFrame);
-          // If the direction of the frame on the edge is opposite to that of the line,
-          // we'll need to drill down to its opposite end, so reverse endOfLine.
-          if (IS_LEVEL_RTL(embeddingLevel) == !lineIsRTL)
+          bool frameIsRTL =
+            (nsBidiPresUtils::FrameDirection(baseFrame) == NSBIDI_RTL);
+          // If the direction of the frame on the edge is opposite to
+          // that of the line, we'll need to drill down to its opposite
+          // end, so reverse endOfLine.
+          if (frameIsRTL != lineIsRTL) {
             endOfLine = !endOfLine;
+          }
         }
       } else {
         it->GetLine(thisLine, &firstFrame, &lineFrameCount, usedRect, &lineFlags);
 
         nsIFrame* frame = firstFrame;
         for (int32_t count = lineFrameCount; count;
              --count, frame = frame->GetNextSibling()) {
           if (!frame->IsGeneratedContentFrame()) {
@@ -6873,18 +6876,18 @@ nsIFrame::GetFrameFromDirection(nsDirect
     nsIFrame *firstFrame;
     nsIFrame *lastFrame;
     if (aVisual && presContext->BidiEnabled()) {
       bool lineIsRTL = it->GetDirection();
       bool isReordered;
       result = it->CheckLineOrder(thisLine, &isReordered, &firstFrame, &lastFrame);
       nsIFrame** framePtr = aDirection == eDirPrevious ? &firstFrame : &lastFrame;
       if (*framePtr) {
-        nsBidiLevel embeddingLevel = nsBidiPresUtils::GetFrameEmbeddingLevel(*framePtr);
-        bool frameIsRTL = IS_LEVEL_RTL(embeddingLevel);
+        bool frameIsRTL =
+          (nsBidiPresUtils::FrameDirection(*framePtr) == NSBIDI_RTL);
         if ((frameIsRTL == lineIsRTL) == (aDirection == eDirPrevious)) {
           nsFrame::GetFirstLeaf(presContext, framePtr);
         } else {
           nsFrame::GetLastLeaf(presContext, framePtr);
         }
         atLineEdge = *framePtr == traversedFrame;
       } else {
         atLineEdge = true;
--- a/layout/generic/nsFrameList.cpp
+++ b/layout/generic/nsFrameList.cpp
@@ -342,37 +342,36 @@ nsFrameList::List(FILE* out) const
 }
 #endif
 
 nsIFrame*
 nsFrameList::GetPrevVisualFor(nsIFrame* aFrame) const
 {
   if (!mFirstChild)
     return nullptr;
-  
+
   nsIFrame* parent = mFirstChild->GetParent();
   if (!parent)
     return aFrame ? aFrame->GetPrevSibling() : LastChild();
 
-  nsBidiLevel baseLevel = nsBidiPresUtils::GetFrameBaseLevel(mFirstChild);  
+  nsBidiDirection paraDir = nsBidiPresUtils::ParagraphDirection(mFirstChild);
 
   nsAutoLineIterator iter = parent->GetLineIterator();
-  if (!iter) { 
+  if (!iter) {
     // Parent is not a block Frame
     if (parent->GetType() == nsGkAtoms::lineFrame) {
       // Line frames are not bidi-splittable, so need to consider bidi reordering
-      if (baseLevel == NSBIDI_LTR) {
+      if (paraDir == NSBIDI_LTR) {
         return nsBidiPresUtils::GetFrameToLeftOf(aFrame, mFirstChild, -1);
       } else { // RTL
         return nsBidiPresUtils::GetFrameToRightOf(aFrame, mFirstChild, -1);
       }
     } else {
       // Just get the next or prev sibling, depending on block and frame direction.
-      nsBidiLevel frameEmbeddingLevel = nsBidiPresUtils::GetFrameEmbeddingLevel(mFirstChild);
-      if (IS_SAME_DIRECTION(frameEmbeddingLevel, baseLevel)) {
+      if (nsBidiPresUtils::IsFrameInParagraphDirection(mFirstChild)) {
         return aFrame ? aFrame->GetPrevSibling() : LastChild();
       } else {
         return aFrame ? aFrame->GetNextSibling() : mFirstChild;
       }
     }
   }
 
   // Parent is a block frame, so use the LineIterator to find the previous visual 
@@ -391,28 +390,28 @@ nsFrameList::GetPrevVisualFor(nsIFrame* 
   nsIFrame* firstFrameOnLine;
   int32_t numFramesOnLine;
   nsRect lineBounds;
   uint32_t lineFlags;
 
   if (aFrame) {
     iter->GetLine(thisLine, &firstFrameOnLine, &numFramesOnLine, lineBounds, &lineFlags);
 
-    if (baseLevel == NSBIDI_LTR) {
+    if (paraDir == NSBIDI_LTR) {
       frame = nsBidiPresUtils::GetFrameToLeftOf(aFrame, firstFrameOnLine, numFramesOnLine);
     } else { // RTL
       frame = nsBidiPresUtils::GetFrameToRightOf(aFrame, firstFrameOnLine, numFramesOnLine);
     }
   }
 
   if (!frame && thisLine > 0) {
     // Get the last frame of the previous line
     iter->GetLine(thisLine - 1, &firstFrameOnLine, &numFramesOnLine, lineBounds, &lineFlags);
 
-    if (baseLevel == NSBIDI_LTR) {
+    if (paraDir == NSBIDI_LTR) {
       frame = nsBidiPresUtils::GetFrameToLeftOf(nullptr, firstFrameOnLine, numFramesOnLine);
     } else { // RTL
       frame = nsBidiPresUtils::GetFrameToRightOf(nullptr, firstFrameOnLine, numFramesOnLine);
     }
   }
   return frame;
 }
 
@@ -421,32 +420,31 @@ nsFrameList::GetNextVisualFor(nsIFrame* 
 {
   if (!mFirstChild)
     return nullptr;
   
   nsIFrame* parent = mFirstChild->GetParent();
   if (!parent)
     return aFrame ? aFrame->GetPrevSibling() : mFirstChild;
 
-  nsBidiLevel baseLevel = nsBidiPresUtils::GetFrameBaseLevel(mFirstChild);
-  
+  nsBidiDirection paraDir = nsBidiPresUtils::ParagraphDirection(mFirstChild);
+
   nsAutoLineIterator iter = parent->GetLineIterator();
   if (!iter) { 
     // Parent is not a block Frame
     if (parent->GetType() == nsGkAtoms::lineFrame) {
       // Line frames are not bidi-splittable, so need to consider bidi reordering
-      if (baseLevel == NSBIDI_LTR) {
+      if (paraDir == NSBIDI_LTR) {
         return nsBidiPresUtils::GetFrameToRightOf(aFrame, mFirstChild, -1);
       } else { // RTL
         return nsBidiPresUtils::GetFrameToLeftOf(aFrame, mFirstChild, -1);
       }
     } else {
       // Just get the next or prev sibling, depending on block and frame direction.
-      nsBidiLevel frameEmbeddingLevel = nsBidiPresUtils::GetFrameEmbeddingLevel(mFirstChild);
-      if (IS_SAME_DIRECTION(frameEmbeddingLevel, baseLevel)) {
+      if (nsBidiPresUtils::IsFrameInParagraphDirection(mFirstChild)) {
         return aFrame ? aFrame->GetNextSibling() : mFirstChild;
       } else {
         return aFrame ? aFrame->GetPrevSibling() : LastChild();
       }
     }
   }
 
   // Parent is a block frame, so use the LineIterator to find the next visual 
@@ -464,30 +462,30 @@ nsFrameList::GetNextVisualFor(nsIFrame* 
   nsIFrame* frame = nullptr;
   nsIFrame* firstFrameOnLine;
   int32_t numFramesOnLine;
   nsRect lineBounds;
   uint32_t lineFlags;
 
   if (aFrame) {
     iter->GetLine(thisLine, &firstFrameOnLine, &numFramesOnLine, lineBounds, &lineFlags);
-    
-    if (baseLevel == NSBIDI_LTR) {
+
+    if (paraDir == NSBIDI_LTR) {
       frame = nsBidiPresUtils::GetFrameToRightOf(aFrame, firstFrameOnLine, numFramesOnLine);
     } else { // RTL
       frame = nsBidiPresUtils::GetFrameToLeftOf(aFrame, firstFrameOnLine, numFramesOnLine);
     }
   }
-  
+
   int32_t numLines = iter->GetNumLines();
   if (!frame && thisLine < numLines - 1) {
     // Get the first frame of the next line
     iter->GetLine(thisLine + 1, &firstFrameOnLine, &numFramesOnLine, lineBounds, &lineFlags);
-    
-    if (baseLevel == NSBIDI_LTR) {
+
+    if (paraDir == NSBIDI_LTR) {
       frame = nsBidiPresUtils::GetFrameToRightOf(nullptr, firstFrameOnLine, numFramesOnLine);
     } else { // RTL
       frame = nsBidiPresUtils::GetFrameToLeftOf(nullptr, firstFrameOnLine, numFramesOnLine);
     }
   }
   return frame;
 }
 
--- a/layout/generic/nsILineIterator.h
+++ b/layout/generic/nsILineIterator.h
@@ -44,16 +44,18 @@ public:
    */
   virtual int32_t GetNumLines() = 0;
 
   /**
    * The prevailing direction of lines.
    *
    * @return true if the CSS direction property for the block is
    *         "rtl", otherwise false
+   *
+   *XXX after bug 924851 change this to return a UBiDiDirection
    */
   virtual bool GetDirection() = 0;
 
   // Return structural information about a line. aFirstFrameOnLine is
   // the first frame on the line and aNumFramesOnLine is the number of
   // frames that are on the line. If the line-number is invalid then
   // aFirstFrameOnLine will be nullptr and aNumFramesOnLine will be
   // zero.
--- a/layout/generic/nsSelection.cpp
+++ b/layout/generic/nsSelection.cpp
@@ -865,27 +865,27 @@ nsFrameSelection::MoveCaret(uint32_t    
   if (NS_FAILED(result) || !frame)
     return NS_FAILED(result) ? result : NS_ERROR_FAILURE;
 
   //set data using mLimiter to stop on scroll views.  If we have a limiter then we stop peeking
   //when we hit scrollable views.  If no limiter then just let it go ahead
   nsPeekOffsetStruct pos(aAmount, eDirPrevious, offsetused, desiredX,
                          true, mLimiter != nullptr, true, aVisualMovement);
 
-  nsBidiLevel baseLevel = nsBidiPresUtils::GetFrameBaseLevel(frame);
-  
+  nsBidiDirection paraDir = nsBidiPresUtils::ParagraphDirection(frame);
+
   CaretAssociateHint tHint(mHint); //temporary variable so we dont set mHint until it is necessary
   switch (aKeycode){
-    case nsIDOMKeyEvent::DOM_VK_RIGHT : 
+    case nsIDOMKeyEvent::DOM_VK_RIGHT :
         InvalidateDesiredX();
-        pos.mDirection = IS_LEVEL_RTL(baseLevel) ? eDirPrevious : eDirNext;
+        pos.mDirection = (paraDir == NSBIDI_RTL) ? eDirPrevious : eDirNext;
       break;
     case nsIDOMKeyEvent::DOM_VK_LEFT :
         InvalidateDesiredX();
-        pos.mDirection = IS_LEVEL_RTL(baseLevel) ? eDirNext : eDirPrevious;
+        pos.mDirection = (paraDir == NSBIDI_RTL) ? eDirNext : eDirPrevious;
       break;
     case nsIDOMKeyEvent::DOM_VK_DELETE :
         InvalidateDesiredX();
         pos.mDirection = eDirNext;
       break;
     case nsIDOMKeyEvent::DOM_VK_BACK_SPACE : 
         InvalidateDesiredX();
         pos.mDirection = eDirPrevious;
@@ -5820,25 +5820,25 @@ Selection::Modify(const nsAString& aAlte
     if (!focusNode) {
       aRv.Throw(NS_ERROR_UNEXPECTED);
       return;
     }
     uint32_t focusOffset = FocusOffset();
     Collapse(focusNode, focusOffset);
   }
 
-  // If the base level of the focused frame is odd, we may have to swap the
-  // direction of the keycode.
+  // If the paragraph direction of the focused frame is right-to-left,
+  // we may have to swap the direction of the keycode.
   nsIFrame *frame;
   int32_t offset;
   rv = GetPrimaryFrameForFocusNode(&frame, &offset, visual);
   if (NS_SUCCEEDED(rv) && frame) {
-    nsBidiLevel baseLevel = nsBidiPresUtils::GetFrameBaseLevel(frame);
-
-    if (IS_LEVEL_RTL(baseLevel)) {
+    nsBidiDirection paraDir = nsBidiPresUtils::ParagraphDirection(frame);
+
+    if (paraDir == NSBIDI_RTL) {
       if (!visual && keycode == nsIDOMKeyEvent::DOM_VK_RIGHT) {
         keycode = nsIDOMKeyEvent::DOM_VK_LEFT;
       }
       else if (!visual && keycode == nsIDOMKeyEvent::DOM_VK_LEFT) {
         keycode = nsIDOMKeyEvent::DOM_VK_RIGHT;
       }
       else if (visual && keycode == nsIDOMKeyEvent::DOM_VK_HOME) {
         keycode = nsIDOMKeyEvent::DOM_VK_END;