Bug 1168891 Part 2 - Allow one caret to be dragged across the other caret. r=mats
authorTing-Yu Lin <tlin@mozilla.com>
Mon, 11 Apr 2016 17:57:29 +0800
changeset 292704 6915c6f22667e848e00db978e34924723c83b3f6
parent 292703 0fed9d8896b2b7300f591eb2ad6fa54822f930c4
child 292705 b71d47c19e79cb7375a388b2c26391de878a4503
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 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
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");