Bug 1168891 Part 2 - Allow one caret to be dragged across the other caret. r=mats

This behavior matches the Android convension and the built-in selection
on all desktop platforms.

MozReview-Commit-ID: 2kNm8UZnqH0

--HG--
extra : rebase_source : 097918c7c49e7cd545af52e9b3f3c540475ec589

b2g/app/b2g.js

@@ -1059,6 +1059,9 @@ pref("layout.accessiblecaret.bar.enabled", true);
 pref("layout.accessiblecaret.use_long_tap_injector", false);
 #endif
 
+// The active caret is disallow to be dragged across the other (inactive) caret.
+pref("layout.accessiblecaret.allow_dragging_across_other_caret", false);
+
 // Enable sync and mozId with Firefox Accounts.
 pref("services.sync.fxaccounts.enabled", true);
 pref("identity.fxaccounts.enabled", true);

layout/base/AccessibleCaretManager.cpp

@@ -78,6 +78,8 @@ AccessibleCaretManager::sCaretsAlwaysTilt = false;
 /*static*/ bool
 AccessibleCaretManager::sCaretsScriptUpdates = false;
 /*static*/ bool
+AccessibleCaretManager::sCaretsAllowDraggingAcrossOtherCaret = true;
+/*static*/ bool
 AccessibleCaretManager::sHapticFeedback = false;
 
 AccessibleCaretManager::AccessibleCaretManager(nsIPresShell* aPresShell)
@@ -104,6 +106,8 @@ AccessibleCaretManager::AccessibleCaretManager(nsIPresShell* aPresShell)
                                  "layout.accessiblecaret.always_tilt");
     Preferences::AddBoolVarCache(&sCaretsScriptUpdates,
       "layout.accessiblecaret.allow_script_change_updates");
+    Preferences::AddBoolVarCache(&sCaretsAllowDraggingAcrossOtherCaret,
+      "layout.accessiblecaret.allow_dragging_across_other_caret", true);
     Preferences::AddBoolVarCache(&sHapticFeedback,
                                  "layout.accessiblecaret.hapticfeedback");
     addedPrefs = true;
@@ -960,6 +964,12 @@ AccessibleCaretManager::RestrictCaretDraggingOffsets(
 
   nsCOMPtr<nsIContent> content = do_QueryInterface(node);
 
+  // Compare the active caret's new position (aOffsets) to the inactive caret's
+  // position.
+  int32_t cmpToInactiveCaretPos =
+    nsContentUtils::ComparePoints(aOffsets.content, aOffsets.StartOffset(),
+                                  content, contentOffset);
+
   // 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,
@@ -974,14 +984,44 @@ AccessibleCaretManager::RestrictCaretDraggingOffsets(
   int32_t cmpToLimit =
     nsContentUtils::ComparePoints(aOffsets.content, aOffsets.StartOffset(),
                                   limit.mResultContent, limit.mContentOffset);
-  if ((mActiveCaret == mFirstCaret.get() && cmpToLimit == 1) ||
+
-      (mActiveCaret == mSecondCaret.get() && cmpToLimit == -1)) {
+  auto SetOffsetsToLimit = [&aOffsets, &limit] () {
-    // 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;
+  };
+
+  if (!sCaretsAllowDraggingAcrossOtherCaret) {
+    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.
+      SetOffsetsToLimit();
+    }
+  } else {
+    switch (cmpToInactiveCaretPos) {
+      case 0:
+        // The active caret's position is the same as the position of the
+        // inactive caret. So set it to the limit to prevent the selection from
+        // being collapsed, resulting in one character being selected.
+        SetOffsetsToLimit();
+        break;
+      case 1:
+        if (mActiveCaret == mFirstCaret.get()) {
+          // First caret was moved across the second caret. After making change
+          // to the selection, the user will drag the second caret.
+          mActiveCaret = mSecondCaret.get();
+        }
+        break;
+      case -1:
+        if (mActiveCaret == mSecondCaret.get()) {
+          // Second caret was moved across the first caret. After making change
+          // to the selection, the user will drag the first caret.
+          mActiveCaret = mFirstCaret.get();
+        }
+        break;
+    }
   }
 
   return true;
@@ -1041,7 +1081,7 @@ AccessibleCaretManager::DragCaretInternal(const nsPoint& aPoint)
 
   nsIFrame::ContentOffsets offsets =
     newFrame->GetContentOffsetsFromPoint(newPoint);
-  if (!offsets.content) {
+  if (offsets.IsNull()) {
     return NS_ERROR_FAILURE;
   }
 
@@ -1144,7 +1184,8 @@ AccessibleCaretManager::AdjustDragBoundary(const nsPoint& aPoint) const
     }
   }
 
-  if (GetCaretMode() == CaretMode::Selection) {
+  if (GetCaretMode() == CaretMode::Selection &&
+      !sCaretsAllowDraggingAcrossOtherCaret) {
     // Bug 1068474: Adjust the Y-coordinate so that the carets won't be in tilt
     // mode when a caret is being dragged surpass the other caret.
     //

layout/base/AccessibleCaretManager.h

@@ -182,12 +182,16 @@ protected:
   // be dragged. Returns the rect relative to aFrame.
   nsRect GetAllChildFrameRectsUnion(nsIFrame* aFrame) const;
 
-  // Suppose the user is dragging the first caret. We do not want it to be
+  // Restrict the active caret's dragging position based on
-  // dragged across the second caret, i.e. we want it to stop at the limit which
+  // sCaretsAllowDraggingAcrossOtherCaret. If the active caret is the first
-  // is the previous character of the second caret. Same rule applies when
+  // caret, the `limit` will be the previous character of the second caret.
-  // dragging the second caret.
+  // Otherwise, the `limit` will be the next character of the first 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.
+  // to the `limit` when 1) sCaretsAllowDraggingAcrossOtherCaret is false and
+  // it's being dragged past the limit. 2) sCaretsAllowDraggingAcrossOtherCaret
+  // is true and the active caret's position is the same as the inactive's
+  // position.
   // @return true if the aOffsets is suitable for changing the selection.
   bool RestrictCaretDraggingOffsets(nsIFrame::ContentOffsets& aOffsets);
 
@@ -297,6 +301,11 @@ protected:
   // carets and ActionBar available.
   static bool sCaretsScriptUpdates;
 
+  // Preference to allow one caret to be dragged across the other caret without
+  // any limitation. When set to false, one caret cannot be dragged across the
+  // other one.
+  static bool sCaretsAllowDraggingAcrossOtherCaret;
+
   // AccessibleCaret pref for haptic feedback behaviour on longPress.
   static bool sHapticFeedback;
 };

layout/base/tests/marionette/test_accessiblecaret_selection_mode.py

@@ -207,6 +207,42 @@ class AccessibleCaretSelectionModeTestCase(MarionetteTestCase):
 
         self.assertEqual(target_content, sel.selected_content)
 
+    @parameterized(_input_id, el_id=_input_id)
+    @parameterized(_textarea_id, el_id=_textarea_id)
+    @parameterized(_textarea_rtl_id, el_id=_textarea_rtl_id)
+    @parameterized(_contenteditable_id, el_id=_contenteditable_id)
+    @parameterized(_content_id, el_id=_content_id)
+    def test_drag_swappable_carets(self, el_id):
+        self.open_test_html(self._selection_html)
+        el = self.marionette.find_element(By.ID, el_id)
+        sel = SelectionManager(el)
+        original_content = sel.content
+        words = original_content.split()
+        self.assertTrue(len(words) >= 1, 'Expect at least one word in the content.')
+
+        target_content1 = words[0]
+        target_content2 = original_content[len(words[0]):]
+
+        # Get the location of the carets at the end of the content for later
+        # use.
+        el.tap()
+        sel.select_all()
+        end_caret_x, end_caret_y = sel.second_caret_location()
+
+        self.long_press_on_word(el, 0)
+
+        # Drag the first caret to the end and back to where it was
+        # immediately. The selection range should not be collapsed.
+        caret1_x, caret1_y = sel.first_caret_location()
+        self.actions.flick(el, caret1_x, caret1_y, end_caret_x, end_caret_y)\
+                    .flick(el, end_caret_x, end_caret_y, caret1_x, caret1_y).perform()
+        self.assertEqual(target_content1, sel.selected_content)
+
+        # Drag the first caret to the end.
+        caret1_x, caret1_y = sel.first_caret_location()
+        self.actions.flick(el, caret1_x, caret1_y, end_caret_x, end_caret_y).perform()
+        self.assertEqual(target_content2, sel.selected_content)
+
     @parameterized(_input_id, el_id=_input_id)
     @parameterized(_textarea_id, el_id=_textarea_id)
     @parameterized(_textarea_rtl_id, el_id=_textarea_rtl_id)
@@ -413,6 +449,29 @@ class AccessibleCaretSelectionModeTestCase(MarionetteTestCase):
         self.assertEqual(self.to_unix_line_ending(sel.selected_content.strip()),
                          '4\nuser can select this 5\nuser')
 
+    def test_drag_swappable_caret_over_non_selectable_field(self):
+        self.open_test_html(self._multiplerange_html)
+        body = self.marionette.find_element(By.ID, 'bd')
+        sel3 = self.marionette.find_element(By.ID, 'sel3')
+        sel4 = self.marionette.find_element(By.ID, 'sel4')
+        sel = SelectionManager(body)
+
+        self.long_press_on_word(sel4, 3)
+        (end_caret1_x, end_caret1_y), (end_caret2_x, end_caret2_y) = sel.carets_location()
+
+        self.long_press_on_word(sel3, 3)
+        (caret1_x, caret1_y), (caret2_x, caret2_y) = sel.carets_location()
+
+        # Drag the first caret down, which will across the second caret.
+        self.actions.flick(body, caret1_x, caret1_y, end_caret1_x, end_caret1_y).perform()
+        self.assertEqual(self.to_unix_line_ending(sel.selected_content.strip()),
+                         '3\nuser can select')
+
+        # The old second caret becomes the first caret. Drag it down again.
+        self.actions.flick(body, caret2_x, caret2_y, end_caret2_x, end_caret2_y).perform()
+        self.assertEqual(self.to_unix_line_ending(sel.selected_content.strip()),
+                         'this')
+
     def test_drag_caret_to_beginning_of_a_line(self):
         '''Bug 1094056
         Test caret visibility when caret is dragged to beginning of a line

modules/libpref/init/all.js

@@ -5018,6 +5018,10 @@ pref("layout.accessiblecaret.always_tilt", false);
 // AccessibleCarets and close UI interaction by default.
 pref("layout.accessiblecaret.allow_script_change_updates", false);
 
+// Allow one caret to be dragged across the other caret without any limitation.
+// This matches the built-in convention for all desktop platforms.
+pref("layout.accessiblecaret.allow_dragging_across_other_caret", true);
+
 // Optionally provide haptic feedback on longPress selection events.
 pref("layout.accessiblecaret.hapticfeedback", false);