Bug 1168891 Part 1 - Refine two functions related to caret positioning. r=mats
authorTing-Yu Lin <tlin@mozilla.com>
Mon, 11 Apr 2016 17:57:29 +0800
changeset 292703 0fed9d8896b2b7300f591eb2ad6fa54822f930c4
parent 292702 1e856020a8d22b6152f18215dbbe7a6a33a77ce4
child 292704 6915c6f22667e848e00db978e34924723c83b3f6
push id74956
push usertlin@mozilla.com
push dateTue, 12 Apr 2016 03:12:39 +0000
treeherdermozilla-inbound@6915c6f22667 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats
bugs1168891
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1168891 Part 1 - Refine two functions related to caret positioning. r=mats FindFirstNodeWithFrame() and CompareRangeWithContentOffset() share a lot of code duplication. I refactor and rename the two functions to improve the readability. MozReview-Commit-ID: CyetLHOGT23
layout/base/AccessibleCaretManager.cpp
layout/base/AccessibleCaretManager.h
--- a/layout/base/AccessibleCaretManager.cpp
+++ b/layout/base/AccessibleCaretManager.cpp
@@ -336,20 +336,22 @@ AccessibleCaretManager::UpdateCaretsForC
 }
 
 void
 AccessibleCaretManager::UpdateCaretsForSelectionMode(UpdateCaretsHint aHint)
 {
   AC_LOG("%s: selection: %p", __FUNCTION__, GetSelection());
 
   int32_t startOffset = 0;
-  nsIFrame* startFrame = FindFirstNodeWithFrame(false, &startOffset);
+  nsIFrame* startFrame =
+    GetFrameForFirstRangeStartOrLastRangeEnd(eDirNext, &startOffset);
 
   int32_t endOffset = 0;
-  nsIFrame* endFrame = FindFirstNodeWithFrame(true, &endOffset);
+  nsIFrame* endFrame =
+    GetFrameForFirstRangeStartOrLastRangeEnd(eDirPrevious, &endOffset);
 
   if (!CompareTreePosition(startFrame, endFrame)) {
     // XXX: Do we really have to hide carets if this condition isn't satisfied?
     HideCarets();
     return;
   }
 
   auto updateSingleCaret = [aHint](AccessibleCaret* aCaret, nsIFrame* aFrame,
@@ -859,149 +861,132 @@ void
 AccessibleCaretManager::FlushLayout() const
 {
   if (mPresShell) {
     mPresShell->FlushPendingNotifications(Flush_Layout);
   }
 }
 
 nsIFrame*
-AccessibleCaretManager::FindFirstNodeWithFrame(bool aBackward,
-                                               int32_t* aOutOffset) const
+AccessibleCaretManager::GetFrameForFirstRangeStartOrLastRangeEnd(
+  nsDirection aDirection, int32_t* aOutOffset, nsINode** aOutNode,
+  int32_t* aOutNodeOffset) const
 {
   if (!mPresShell) {
     return nullptr;
   }
 
+  MOZ_ASSERT(GetCaretMode() == CaretMode::Selection);
+
+  nsRange* range = nullptr;
+  RefPtr<nsINode> startNode;
+  RefPtr<nsINode> endNode;
+  int32_t nodeOffset = 0;
+  CaretAssociationHint hint;
+
   RefPtr<Selection> selection = GetSelection();
-  if (!selection) {
-    return nullptr;
-  }
+  bool findInFirstRangeStart = aDirection == eDirNext;
 
-  RefPtr<nsFrameSelection> fs = GetFrameSelection();
-  if (!fs) {
-    return nullptr;
+  if (findInFirstRangeStart) {
+    range = selection->GetRangeAt(0);
+    startNode = range->GetStartParent();
+    endNode = range->GetEndParent();
+    nodeOffset = range->StartOffset();
+    hint = CARET_ASSOCIATE_AFTER;
+  } else {
+    range = selection->GetRangeAt(selection->RangeCount() - 1);
+    startNode = range->GetEndParent();
+    endNode = range->GetStartParent();
+    nodeOffset = range->EndOffset();
+    hint = CARET_ASSOCIATE_BEFORE;
   }
 
-  uint32_t rangeCount = selection->RangeCount();
-  if (rangeCount <= 0) {
-    return nullptr;
-  }
-
-  nsRange* range = selection->GetRangeAt(aBackward ? rangeCount - 1 : 0);
-  RefPtr<nsINode> startNode =
-    aBackward ? range->GetEndParent() : range->GetStartParent();
-  RefPtr<nsINode> endNode =
-    aBackward ? range->GetStartParent() : range->GetEndParent();
-  int32_t offset = aBackward ? range->EndOffset() : range->StartOffset();
   nsCOMPtr<nsIContent> startContent = do_QueryInterface(startNode);
-  CaretAssociationHint hintStart =
-    aBackward ? CARET_ASSOCIATE_BEFORE : CARET_ASSOCIATE_AFTER;
+  RefPtr<nsFrameSelection> fs = GetFrameSelection();
   nsIFrame* startFrame =
-    fs->GetFrameForNodeOffset(startContent, offset, hintStart, aOutOffset);
+    fs->GetFrameForNodeOffset(startContent, nodeOffset, hint, aOutOffset);
 
   if (startFrame) {
+    if (aOutNode) {
+      *aOutNode = startNode.get();
+    }
+    if (aOutNodeOffset) {
+      *aOutNodeOffset = nodeOffset;
+    }
     return startFrame;
   }
 
   ErrorResult err;
   RefPtr<TreeWalker> walker = mPresShell->GetDocument()->CreateTreeWalker(
     *startNode, nsIDOMNodeFilter::SHOW_ALL, nullptr, err);
 
   if (!walker) {
     return nullptr;
   }
 
   startFrame = startContent ? startContent->GetPrimaryFrame() : nullptr;
   while (!startFrame && startNode != endNode) {
-    startNode = aBackward ? walker->PreviousNode(err) : walker->NextNode(err);
+    startNode = findInFirstRangeStart ? walker->NextNode(err)
+                                      : walker->PreviousNode(err);
 
     if (!startNode) {
       break;
     }
 
     startContent = startNode->AsContent();
     startFrame = startContent ? startContent->GetPrimaryFrame() : nullptr;
   }
   return startFrame;
 }
 
 bool
-AccessibleCaretManager::CompareRangeWithContentOffset(nsIFrame::ContentOffsets& aOffsets)
+AccessibleCaretManager::RestrictCaretDraggingOffsets(
+  nsIFrame::ContentOffsets& aOffsets)
 {
-  Selection* selection = GetSelection();
-  if (!selection) {
+  if (!mPresShell) {
+    return false;
+  }
+
+  MOZ_ASSERT(GetCaretMode() == CaretMode::Selection);
+
+  nsDirection dir = mActiveCaret == mFirstCaret.get() ? eDirPrevious : eDirNext;
+  int32_t offset = 0;
+  nsINode* node = nullptr;
+  int32_t contentOffset = 0;
+  nsIFrame* frame =
+    GetFrameForFirstRangeStartOrLastRangeEnd(dir, &offset, &node, &contentOffset);
+
+  if (!frame) {
     return false;
   }
 
-  uint32_t rangeCount = selection->RangeCount();
-  MOZ_ASSERT(rangeCount > 0);
-
-  int32_t rangeIndex = (mActiveCaret == mFirstCaret.get() ? rangeCount - 1 : 0);
-  RefPtr<nsRange> range = selection->GetRangeAt(rangeIndex);
-
-  nsINode* node = nullptr;
-  int32_t nodeOffset = 0;
-  CaretAssociationHint hint;
-  nsDirection dir;
-
-  if (mActiveCaret == mFirstCaret.get()) {
-    // Check previous character of end node offset
-    node = range->GetEndParent();
-    nodeOffset = range->EndOffset();
-    hint = CARET_ASSOCIATE_BEFORE;
-    dir = eDirPrevious;
-  } else {
-    // Check next character of start node offset
-    node = range->GetStartParent();
-    nodeOffset = range->StartOffset();
-    hint = CARET_ASSOCIATE_AFTER;
-    dir = eDirNext;
-  }
   nsCOMPtr<nsIContent> content = do_QueryInterface(node);
 
-  RefPtr<nsFrameSelection> fs = GetFrameSelection();
-  if (!fs) {
-    return false;
+  // Move one character (in the direction of dir) from the inactive caret's
+  // position. This is the limit for the active caret's new position.
+  nsPeekOffsetStruct limit(eSelectCluster, dir, offset, nsPoint(0, 0), true, true,
+                           false, false, false);
+  nsresult rv = frame->PeekOffset(&limit);
+  if (NS_FAILED(rv)) {
+    limit.mResultContent = content;
+    limit.mContentOffset = contentOffset;
   }
 
-  int32_t offset = 0;
-  nsIFrame* theFrame =
-    fs->GetFrameForNodeOffset(content, nodeOffset, hint, &offset);
-
-  if (!theFrame) {
-    return false;
-  }
-
-  // Move one character forward/backward from point and get offset
-  nsPeekOffsetStruct pos(eSelectCluster,
-                         dir,
-                         offset,
-                         nsPoint(0, 0),
-                         true,
-                         true,  //limit on scrolled views
-                         false,
-                         false,
-                         false);
-  nsresult rv = theFrame->PeekOffset(&pos);
-  if (NS_FAILED(rv)) {
-    pos.mResultContent = content;
-    pos.mContentOffset = nodeOffset;
-  }
-
-  // Compare with current point
-  int32_t result = nsContentUtils::ComparePoints(aOffsets.content,
-                                                 aOffsets.StartOffset(),
-                                                 pos.mResultContent,
-                                                 pos.mContentOffset);
-  if ((mActiveCaret == mFirstCaret.get() && result == 1) ||
-      (mActiveCaret == mSecondCaret.get() && result == -1)) {
-    aOffsets.content = pos.mResultContent;
-    aOffsets.offset = pos.mContentOffset;
-    aOffsets.secondaryOffset = pos.mContentOffset;
+  // Compare the active caret's new position (aOffsets) to the limit.
+  int32_t cmpToLimit =
+    nsContentUtils::ComparePoints(aOffsets.content, aOffsets.StartOffset(),
+                                  limit.mResultContent, limit.mContentOffset);
+  if ((mActiveCaret == mFirstCaret.get() && cmpToLimit == 1) ||
+      (mActiveCaret == mSecondCaret.get() && cmpToLimit == -1)) {
+    // The active caret's position is past the limit, which we don't allow
+    // here. So set it to the limit, resulting in one character being
+    // selected.
+    aOffsets.content = limit.mResultContent;
+    aOffsets.offset = limit.mContentOffset;
+    aOffsets.secondaryOffset = limit.mContentOffset;
   }
 
   return true;
 }
 
 bool
 AccessibleCaretManager::CompareTreePosition(nsIFrame* aStartFrame,
                                             nsIFrame* aEndFrame) const
@@ -1061,17 +1046,17 @@ AccessibleCaretManager::DragCaretInterna
   }
 
   Selection* selection = GetSelection();
   if (!selection) {
     return NS_ERROR_NULL_POINTER;
   }
 
   if (GetCaretMode() == CaretMode::Selection &&
-      !CompareRangeWithContentOffset(offsets)) {
+      !RestrictCaretDraggingOffsets(offsets)) {
     return NS_ERROR_FAILURE;
   }
 
   ClearMaintainedSelection();
 
   nsIFrame* anchorFrame = nullptr;
   selection->GetPrimaryFrameForAnchorNode(&anchorFrame);
 
--- a/layout/base/AccessibleCaretManager.h
+++ b/layout/base/AccessibleCaretManager.h
@@ -152,20 +152,23 @@ protected:
   // Change focus to aFrame if it isn't nullptr. Otherwise, clear the old focus
   // then re-focus the window.
   void ChangeFocusToOrClearOldFocus(nsIFrame* aFrame) const;
 
   nsresult SelectWord(nsIFrame* aFrame, const nsPoint& aPoint) const;
   void SetSelectionDragState(bool aState) const;
   void SetSelectionDirection(nsDirection aDir) const;
 
-  // If aBackward is false, find the first node from the first range in current
-  // selection, and return the frame and the offset into that frame. If aBackward
-  // is true, find the last node from the last range instead.
-  nsIFrame* FindFirstNodeWithFrame(bool aBackward, int32_t* aOutOffset) const;
+  // If aDirection is eDirNext, get the frame for the range start in the first
+  // range from the current selection, and return the offset into that frame as
+  // well as the range start node and the node offset. Otherwise, get the frame
+  // and offset for the range end in the last range instead.
+  nsIFrame* GetFrameForFirstRangeStartOrLastRangeEnd(
+    nsDirection aDirection, int32_t* aOutOffset, nsINode** aOutNode = nullptr,
+    int32_t* aOutNodeOffset = nullptr) const;
 
   nsresult DragCaretInternal(const nsPoint& aPoint);
   nsPoint AdjustDragBoundary(const nsPoint& aPoint) const;
   void ClearMaintainedSelection() const;
 
   // Caller is responsible to use IsTerminated() to check whether PresShell is
   // still valid.
   void FlushLayout() const;
@@ -174,21 +177,24 @@ protected:
   dom::Selection* GetSelection() const;
   already_AddRefed<nsFrameSelection> GetFrameSelection() const;
 
   // Get the union of all the child frame scrollable overflow rects for aFrame,
   // which is used as a helper function to restrict the area where the caret can
   // be dragged. Returns the rect relative to aFrame.
   nsRect GetAllChildFrameRectsUnion(nsIFrame* aFrame) const;
 
-  // If we're dragging the first caret, we do not want to drag it over the
-  // previous character of the second caret. Same as the second caret. So we
-  // check if content offset exceeds the previous/next character of second/first
-  // caret base the active caret.
-  bool CompareRangeWithContentOffset(nsIFrame::ContentOffsets& aOffsets);
+  // Suppose the user is dragging the first caret. We do not want it to be
+  // dragged across the second caret, i.e. we want it to stop at the limit which
+  // is the previous character of the second caret. Same rule applies when
+  // dragging the second caret.
+  // @param aOffsets is the new position of the active caret, and it will be set
+  // to the limit if it's being dragged past the limit.
+  // @return true if the aOffsets is suitable for changing the selection.
+  bool RestrictCaretDraggingOffsets(nsIFrame::ContentOffsets& aOffsets);
 
   // Timeout in milliseconds to hide the AccessibleCaret under cursor mode while
   // no one touches it.
   uint32_t CaretTimeoutMs() const;
   void LaunchCaretTimeoutTimer();
   void CancelCaretTimeoutTimer();
 
   // ---------------------------------------------------------------------------