Bug 1168891 Part 2 - Allow one caret to be dragged across the other caret. draft
authorTing-Yu Lin <tlin@mozilla.com>
Mon, 11 Apr 2016 17:57:29 +0800
changeset 349386 514e867ae504bde25d22a7007279a6e66dedde9a
parent 349385 779da1b63e611f26e3e52d35e99d5c6a9069328e
child 518081 a15f8a06fce38dab2d80e0e7a8e60b0ecfb43ab7
push id15066
push usertlin@mozilla.com
push dateMon, 11 Apr 2016 09:59:34 +0000
bugs1168891
milestone48.0a1
Bug 1168891 Part 2 - Allow one caret to be dragged across the other caret. This behavior matches the Android convension and the built-in selection on all desktop platforms. MozReview-Commit-ID: 2kNm8UZnqH0
b2g/app/b2g.js
layout/base/AccessibleCaretManager.cpp
layout/base/AccessibleCaretManager.h
layout/base/tests/marionette/test_accessiblecaret_selection_mode.py
modules/libpref/init/all.js
--- a/b2g/app/b2g.js
+++ b/b2g/app/b2g.js
@@ -1054,16 +1054,19 @@ pref("layout.accessiblecaret.enabled", t
 // by the spec in bug 921965.
 pref("layout.accessiblecaret.bar.enabled", true);
 
 // APZ on real devices supports long tap events.
 #ifdef MOZ_WIDGET_GONK
 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);
 
 // Mobile Identity API.
 pref("services.mobileid.server.uri", "https://msisdn.services.mozilla.com");
 
 pref("identity.fxaccounts.remote.oauth.uri", "https://oauth.accounts.firefox.com/v1");
--- a/layout/base/AccessibleCaretManager.cpp
+++ b/layout/base/AccessibleCaretManager.cpp
@@ -73,16 +73,18 @@ AccessibleCaretManager::sSelectionBarEna
 AccessibleCaretManager::sCaretShownWhenLongTappingOnEmptyContent = false;
 /*static*/ bool
 AccessibleCaretManager::sCaretsExtendedVisibility = false;
 /*static*/ bool
 AccessibleCaretManager::sCaretsAlwaysTilt = false;
 /*static*/ bool
 AccessibleCaretManager::sCaretsScriptUpdates = false;
 /*static*/ bool
+AccessibleCaretManager::sCaretsAllowDraggingAcrossOtherCaret = true;
+/*static*/ bool
 AccessibleCaretManager::sHapticFeedback = false;
 
 AccessibleCaretManager::AccessibleCaretManager(nsIPresShell* aPresShell)
   : mPresShell(aPresShell)
 {
   if (!mPresShell) {
     return;
   }
@@ -99,16 +101,18 @@ AccessibleCaretManager::AccessibleCaretM
     Preferences::AddBoolVarCache(&sCaretShownWhenLongTappingOnEmptyContent,
       "layout.accessiblecaret.caret_shown_when_long_tapping_on_empty_content");
     Preferences::AddBoolVarCache(&sCaretsExtendedVisibility,
                                  "layout.accessiblecaret.extendedvisibility");
     Preferences::AddBoolVarCache(&sCaretsAlwaysTilt,
                                  "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;
   }
 }
 
 AccessibleCaretManager::~AccessibleCaretManager()
 {
@@ -955,38 +959,74 @@ AccessibleCaretManager::RestrictCaretDra
     GetFrameForFirstRangeStartOrLastRangeEnd(dir, &offset, &node, &contentOffset);
 
   if (!frame) {
     return false;
   }
 
   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,
                            false, false, false);
   nsresult rv = frame->PeekOffset(&limit);
   if (NS_FAILED(rv)) {
     limit.mResultContent = content;
     limit.mContentOffset = contentOffset;
   }
 
   // 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.
+
+  auto SetOffsetsToLimit = [&aOffsets, &limit] () {
     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;
 }
 
 bool
 AccessibleCaretManager::CompareTreePosition(nsIFrame* aStartFrame,
                                             nsIFrame* aEndFrame) const
@@ -1036,17 +1076,17 @@ AccessibleCaretManager::DragCaretInterna
   bool selectable;
   newFrame->IsSelectable(&selectable, nullptr);
   if (!selectable) {
     return NS_ERROR_FAILURE;
   }
 
   nsIFrame::ContentOffsets offsets =
     newFrame->GetContentOffsetsFromPoint(newPoint);
-  if (!offsets.content) {
+  if (offsets.IsNull()) {
     return NS_ERROR_FAILURE;
   }
 
   Selection* selection = GetSelection();
   if (!selection) {
     return NS_ERROR_NULL_POINTER;
   }
 
@@ -1139,17 +1179,18 @@ AccessibleCaretManager::AdjustDragBounda
 
       // Shrink the rect to make sure we never hit the boundary.
       boundary.Deflate(kBoundaryAppUnits);
 
       adjustedPoint = boundary.ClampPoint(adjustedPoint);
     }
   }
 
-  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.
     //
     // For example, when dragging the second caret, the horizontal boundary (lower
     // bound) of its Y-coordinate is the logical position of the first caret.
     // Likewise, when dragging the first caret, the horizontal boundary (upper
     // bound) of its Y-coordinate is the logical position of the second caret.
     if (mActiveCaret == mFirstCaret.get()) {
--- a/layout/base/AccessibleCaretManager.h
+++ b/layout/base/AccessibleCaretManager.h
@@ -177,22 +177,26 @@ 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;
 
-  // 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.
+  // Restrict the active caret's dragging position based on
+  // sCaretsAllowDraggingAcrossOtherCaret. If the active caret is the first
+  // caret, the `limit` will be the previous character of 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);
 
   // Timeout in milliseconds to hide the AccessibleCaret under cursor mode while
   // no one touches it.
   uint32_t CaretTimeoutMs() const;
   void LaunchCaretTimeoutTimer();
   void CancelCaretTimeoutTimer();
@@ -292,16 +296,21 @@ protected:
   // carets become tilt only when they are overlapping.
   static bool sCaretsAlwaysTilt;
 
   // By default, javascript content selection changes closes AccessibleCarets and
   // UI interactions. Optionally, we can try to maintain the active UI, keeping
   // 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;
 };
 
 std::ostream& operator<<(std::ostream& aStream,
                          const AccessibleCaretManager::CaretMode& aCaretMode);
 
 std::ostream& operator<<(std::ostream& aStream,
--- a/layout/base/tests/marionette/test_accessiblecaret_selection_mode.py
+++ b/layout/base/tests/marionette/test_accessiblecaret_selection_mode.py
@@ -207,16 +207,52 @@ class AccessibleCaretSelectionModeTestCa
 
         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)
+    @parameterized(_contenteditable_id, el_id=_contenteditable_id)
+    @parameterized(_content_id, el_id=_content_id)
     def test_minimum_select_one_character(self, el_id):
         self.open_test_html(self._selection_html)
         el = self.marionette.find_element(By.ID, el_id)
         self._test_minimum_select_one_character(el)
 
     @parameterized(_textarea2_id, el_id=_textarea2_id)
     @parameterized(_contenteditable2_id, el_id=_contenteditable2_id)
     @parameterized(_content2_id, el_id=_content2_id)
@@ -408,16 +444,39 @@ class AccessibleCaretSelectionModeTestCa
                          'this 3\nuser can select this 4\nuser can select this 5\nuser')
 
         # Drag first caret to target location
         (caret1_x, caret1_y), (caret2_x, caret2_y) = sel.carets_location()
         self.actions.flick(body, caret1_x, caret1_y, end_caret_x, end_caret_y, 1).perform()
         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
         '''
         self.open_test_html(self._multiplerange_html)
         body = self.marionette.find_element(By.ID, 'bd')
         sel1 = self.marionette.find_element(By.ID, 'sel1')
         sel2 = self.marionette.find_element(By.ID, 'sel2')
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -5013,16 +5013,20 @@ pref("layout.accessiblecaret.extendedvis
 
 // By default, carets become tilt only when they are overlapping.
 pref("layout.accessiblecaret.always_tilt", false);
 
 // Selection change notifications generated by Javascript hide
 // 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);
 
 // Wakelock is disabled by default.
 pref("dom.wakelock.enabled", false);
 
 // The URL of the Firefox Accounts auth server backend
 pref("identity.fxaccounts.auth.uri", "https://api.accounts.firefox.com/v1");