Bug 1168891 Part 1 - Refine two functions related to caret positioning. draft
authorTing-Yu Lin <tlin@mozilla.com>
Mon, 11 Apr 2016 17:57:29 +0800
changeset 349385 779da1b63e611f26e3e52d35e99d5c6a9069328e
parent 349384 e847cfcb315f511f4928b03fd47dcf57aad05e1e
child 349386 514e867ae504bde25d22a7007279a6e66dedde9a
push id15066
push usertlin@mozilla.com
push dateMon, 11 Apr 2016 09:59:34 +0000
bugs1168891
milestone48.0a1
Bug 1168891 Part 1 - Refine two functions related to caret positioning. 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();
 
   // ---------------------------------------------------------------------------