Bug 1347049 - Remove the ability to hide AccessibleCaret when scrolling. r?mats draft
authorTing-Yu Lin <aethanyc@gmail.com>
Wed, 04 Apr 2018 15:45:52 +0800
changeset 777212 592e884d76f87a1be989c291aa8efb1192df90e8
parent 776948 00bdc9451be6557ccce1492b9b966d4435615380
push id105111
push userbmo:aethanyc@gmail.com
push dateWed, 04 Apr 2018 12:26:49 +0000
reviewersmats
bugs1347049
milestone61.0a1
Bug 1347049 - Remove the ability to hide AccessibleCaret when scrolling. r?mats This feature is B2G only. Remove it to make AccessibleCaret simpler to maintain. MozReview-Commit-ID: 7JZw5XtaUeU
layout/base/AccessibleCaretManager.cpp
layout/base/AccessibleCaretManager.h
layout/base/gtest/TestAccessibleCaretManager.cpp
modules/libpref/init/all.js
--- a/layout/base/AccessibleCaretManager.cpp
+++ b/layout/base/AccessibleCaretManager.cpp
@@ -73,18 +73,16 @@ std::ostream& operator<<(std::ostream& a
 
 /* static */ bool
 AccessibleCaretManager::sSelectionBarEnabled = false;
 /* static */ bool
 AccessibleCaretManager::sCaretShownWhenLongTappingOnEmptyContent = false;
 /* static */ bool
 AccessibleCaretManager::sCaretsAlwaysTilt = false;
 /* static */ bool
-AccessibleCaretManager::sCaretsAlwaysShowWhenScrolling = true;
-/* static */ bool
 AccessibleCaretManager::sCaretsScriptUpdates = false;
 /* static */ bool
 AccessibleCaretManager::sCaretsAllowDraggingAcrossOtherCaret = true;
 /* static */ bool
 AccessibleCaretManager::sHapticFeedback = false;
 /* static */ bool
 AccessibleCaretManager::sExtendSelectionForPhoneNumber = false;
 /* static */ bool
@@ -103,18 +101,16 @@ AccessibleCaretManager::AccessibleCaretM
   static bool addedPrefs = false;
   if (!addedPrefs) {
     Preferences::AddBoolVarCache(&sSelectionBarEnabled,
                                  "layout.accessiblecaret.bar.enabled");
     Preferences::AddBoolVarCache(&sCaretShownWhenLongTappingOnEmptyContent,
       "layout.accessiblecaret.caret_shown_when_long_tapping_on_empty_content");
     Preferences::AddBoolVarCache(&sCaretsAlwaysTilt,
                                  "layout.accessiblecaret.always_tilt");
-    Preferences::AddBoolVarCache(&sCaretsAlwaysShowWhenScrolling,
-      "layout.accessiblecaret.always_show_when_scrolling", true);
     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");
     Preferences::AddBoolVarCache(&sExtendSelectionForPhoneNumber,
       "layout.accessiblecaret.extend_selection_for_phone_number");
@@ -647,47 +643,32 @@ AccessibleCaretManager::SelectWordOrShor
 
 void
 AccessibleCaretManager::OnScrollStart()
 {
   AC_LOG("%s", __FUNCTION__);
 
   mIsScrollStarted = true;
 
-  if (!sCaretsAlwaysShowWhenScrolling) {
-    // Backup the appearance so that we can restore them after the scrolling
-    // ends.
-    mFirstCaretAppearanceOnScrollStart = mFirstCaret->GetAppearance();
-    mSecondCaretAppearanceOnScrollStart = mSecondCaret->GetAppearance();
-    HideCarets();
-    return;
-  }
-
   if (mFirstCaret->IsLogicallyVisible() || mSecondCaret->IsLogicallyVisible()) {
     // Dispatch the event only if one of the carets is logically visible like in
     // HideCarets().
     DispatchCaretStateChangedEvent(CaretChangedReason::Scroll);
   }
 }
 
 void
 AccessibleCaretManager::OnScrollEnd()
 {
   if (mLastUpdateCaretMode != GetCaretMode()) {
     return;
   }
 
   mIsScrollStarted = false;
 
-  if (!sCaretsAlwaysShowWhenScrolling) {
-    // Restore the appearance which is saved before the scrolling is started.
-    mFirstCaret->SetAppearance(mFirstCaretAppearanceOnScrollStart);
-    mSecondCaret->SetAppearance(mSecondCaretAppearanceOnScrollStart);
-  }
-
   if (GetCaretMode() == CaretMode::Cursor) {
     if (!mFirstCaret->IsLogicallyVisible()) {
       // If the caret is hidden (Appearance::None) due to blur, no
       // need to update it.
       return;
     }
   }
 
--- a/layout/base/AccessibleCaretManager.h
+++ b/layout/base/AccessibleCaretManager.h
@@ -305,23 +305,16 @@ protected:
   UniquePtr<AccessibleCaret> mSecondCaret;
 
   // The caret being pressed or dragged.
   AccessibleCaret* mActiveCaret = nullptr;
 
   // The caret mode since last update carets.
   CaretMode mLastUpdateCaretMode = CaretMode::None;
 
-  // Store the appearance of the carets when calling OnScrollStart() so that it
-  // can be restored in OnScrollEnd().
-  AccessibleCaret::Appearance mFirstCaretAppearanceOnScrollStart =
-                                 AccessibleCaret::Appearance::None;
-  AccessibleCaret::Appearance mSecondCaretAppearanceOnScrollStart =
-                                 AccessibleCaret::Appearance::None;
-
   // The last input source that the event hub saw. We use this to decide whether
   // or not show the carets when the selection is updated, as we want to hide
   // the carets for mouse-triggered selection changes but show them for other
   // input types such as touch.
   uint16_t mLastInputSource = dom::MouseEventBinding::MOZ_SOURCE_UNKNOWN;
 
   // Set to true in OnScrollStart() and set to false in OnScrollEnd().
   bool mIsScrollStarted = false;
@@ -350,21 +343,16 @@ protected:
   // which is based on the emptiness of the content, into something more
   // heuristic. See UpdateCaretsForCursorMode() for the details.
   static bool sCaretShownWhenLongTappingOnEmptyContent;
 
   // Preference to make carets always tilt in selection mode. By default, the
   // carets become tilt only when they are overlapping.
   static bool sCaretsAlwaysTilt;
 
-  // Preference to allow carets always show when scrolling (either panning or
-  // zooming) the page. When set to false, carets will hide during scrolling,
-  // and show again after the user lifts the finger off the screen.
-  static bool sCaretsAlwaysShowWhenScrolling;
-
   // 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.
--- a/layout/base/gtest/TestAccessibleCaretManager.cpp
+++ b/layout/base/gtest/TestAccessibleCaretManager.cpp
@@ -56,17 +56,16 @@ public:
   class MockAccessibleCaretManager : public AccessibleCaretManager
   {
   public:
     using CaretMode = AccessibleCaretManager::CaretMode;
     using AccessibleCaretManager::UpdateCarets;
     using AccessibleCaretManager::HideCarets;
     using AccessibleCaretManager::sCaretShownWhenLongTappingOnEmptyContent;
     using AccessibleCaretManager::sCaretsAlwaysTilt;
-    using AccessibleCaretManager::sCaretsAlwaysShowWhenScrolling;
 
     MockAccessibleCaretManager()
       : AccessibleCaretManager(nullptr)
     {
       mFirstCaret = MakeUnique<MockAccessibleCaret>();
       mSecondCaret = MakeUnique<MockAccessibleCaret>();
     }
 
@@ -349,86 +348,91 @@ TEST_F(AccessibleCaretManagerTester, Tes
 
   mManager.OnScrollPositionChanged();
   EXPECT_EQ(FirstCaretAppearance(), Appearance::None);
 }
 
 TEST_F(AccessibleCaretManagerTester, TestScrollInSelectionMode)
 MOZ_CAN_RUN_SCRIPT
 {
-  // Simulate caret hiding when scrolling.
-  AutoRestore<bool> savesCaretsAlwaysShowWhenScrolling(
-    MockAccessibleCaretManager::sCaretsAlwaysShowWhenScrolling);
-  MockAccessibleCaretManager::sCaretsAlwaysShowWhenScrolling = false;
-
   EXPECT_CALL(mManager, GetCaretMode())
     .WillRepeatedly(Return(CaretMode::Selection));
 
   MockFunction<void(std::string aCheckPointName)> check;
   {
     InSequence dummy;
 
     // Initially, first caret is out of scrollport, and second caret is visible.
     EXPECT_CALL(mManager.FirstCaret(), SetPosition(_, _))
       .WillOnce(Return(PositionChangedResult::Invisible));
 
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
                   CaretChangedReason::Updateposition));
     EXPECT_CALL(check, Call("updatecarets"));
 
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
-                  CaretChangedReason::Visibilitychange));
+                  CaretChangedReason::Scroll));
     EXPECT_CALL(check, Call("scrollstart1"));
 
+    EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
+                  CaretChangedReason::Updateposition));
+    EXPECT_CALL(check, Call("reflow1"));
+
     // After scroll ended, first caret is visible and second caret is out of
     // scroll port.
     EXPECT_CALL(mManager.SecondCaret(), SetPosition(_, _))
       .WillOnce(Return(PositionChangedResult::Invisible));
 
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
                   CaretChangedReason::Updateposition));
     EXPECT_CALL(check, Call("scrollend1"));
 
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
-                  CaretChangedReason::Visibilitychange));
+                  CaretChangedReason::Scroll));
     EXPECT_CALL(check, Call("scrollstart2"));
 
+    EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
+                  CaretChangedReason::Updateposition));
+    EXPECT_CALL(check, Call("reflow2"));
+
     // After the scroll ended, both carets are visible.
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
                   CaretChangedReason::Updateposition));
     EXPECT_CALL(check, Call("scrollend2"));
   }
 
   mManager.UpdateCarets();
   EXPECT_EQ(FirstCaretAppearance(), Appearance::NormalNotShown);
   EXPECT_EQ(SecondCaretAppearance(), Appearance::Normal);
   check.Call("updatecarets");
 
   mManager.OnScrollStart();
-  EXPECT_EQ(FirstCaretAppearance(), Appearance::None);
-  EXPECT_EQ(SecondCaretAppearance(), Appearance::None);
+  EXPECT_EQ(FirstCaretAppearance(), Appearance::NormalNotShown);
+  EXPECT_EQ(SecondCaretAppearance(), Appearance::Normal);
   check.Call("scrollstart1");
 
   mManager.OnReflow();
-  EXPECT_EQ(FirstCaretAppearance(), Appearance::None);
-  EXPECT_EQ(SecondCaretAppearance(), Appearance::None);
+  EXPECT_EQ(FirstCaretAppearance(), Appearance::NormalNotShown);
+  EXPECT_EQ(SecondCaretAppearance(), Appearance::Normal);
+  check.Call("reflow1");
 
   mManager.OnScrollEnd();
   EXPECT_EQ(FirstCaretAppearance(), Appearance::Normal);
   EXPECT_EQ(SecondCaretAppearance(), Appearance::NormalNotShown);
   check.Call("scrollend1");
 
   mManager.OnScrollStart();
-  EXPECT_EQ(FirstCaretAppearance(), Appearance::None);
-  EXPECT_EQ(SecondCaretAppearance(), Appearance::None);
+  EXPECT_EQ(FirstCaretAppearance(), Appearance::Normal);
+  EXPECT_EQ(SecondCaretAppearance(), Appearance::NormalNotShown);
   check.Call("scrollstart2");
 
   mManager.OnReflow();
-  EXPECT_EQ(FirstCaretAppearance(), Appearance::None);
-  EXPECT_EQ(SecondCaretAppearance(), Appearance::None);
+  EXPECT_EQ(FirstCaretAppearance(), Appearance::Normal);
+  EXPECT_EQ(SecondCaretAppearance(), Appearance::NormalNotShown);
+  check.Call("reflow2");
 
   mManager.OnScrollEnd();
   EXPECT_EQ(FirstCaretAppearance(), Appearance::Normal);
   EXPECT_EQ(SecondCaretAppearance(), Appearance::Normal);
   check.Call("scrollend2");
 }
 
 TEST_F(AccessibleCaretManagerTester,
@@ -536,87 +540,77 @@ MOZ_CAN_RUN_SCRIPT
   EXPECT_EQ(FirstCaretAppearance(), Appearance::Left);
   EXPECT_EQ(SecondCaretAppearance(), Appearance::Right);
   check.Call("scrollend2");
 }
 
 TEST_F(AccessibleCaretManagerTester, TestScrollInCursorModeWhenLogicallyVisible)
 MOZ_CAN_RUN_SCRIPT
 {
-  // Simulate caret hiding when scrolling.
-  AutoRestore<bool> savesCaretsAlwaysShowWhenScrolling(
-    MockAccessibleCaretManager::sCaretsAlwaysShowWhenScrolling);
-  MockAccessibleCaretManager::sCaretsAlwaysShowWhenScrolling = false;
-
   EXPECT_CALL(mManager, GetCaretMode())
     .WillRepeatedly(Return(CaretMode::Cursor));
 
   EXPECT_CALL(mManager, HasNonEmptyTextContent(_))
     .WillRepeatedly(Return(true));
 
   MockFunction<void(std::string aCheckPointName)> check;
   {
     InSequence dummy;
 
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
                   CaretChangedReason::Updateposition)).Times(1);
     EXPECT_CALL(check, Call("updatecarets"));
 
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
-                  CaretChangedReason::Visibilitychange)).Times(1);
+                  CaretChangedReason::Scroll)).Times(1);
     EXPECT_CALL(check, Call("scrollstart1"));
 
     // After scroll ended, the caret is out of scroll port.
     EXPECT_CALL(mManager.FirstCaret(), SetPosition(_, _))
       .WillRepeatedly(Return(PositionChangedResult::Invisible));
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
                   CaretChangedReason::Updateposition)).Times(1);
     EXPECT_CALL(check, Call("scrollend1"));
 
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
-                  CaretChangedReason::Visibilitychange)).Times(1);
+                  CaretChangedReason::Scroll)).Times(1);
     EXPECT_CALL(check, Call("scrollstart2"));
 
     // After scroll ended, the caret is visible again.
     EXPECT_CALL(mManager.FirstCaret(), SetPosition(_, _))
       .WillRepeatedly(Return(PositionChangedResult::Changed));
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
                   CaretChangedReason::Updateposition)).Times(1);
     EXPECT_CALL(check, Call("scrollend2"));
   }
 
   mManager.UpdateCarets();
   EXPECT_EQ(FirstCaretAppearance(), Appearance::Normal);
   check.Call("updatecarets");
 
   mManager.OnScrollStart();
-  EXPECT_EQ(FirstCaretAppearance(), Appearance::None);
+  EXPECT_EQ(FirstCaretAppearance(), Appearance::Normal);
   check.Call("scrollstart1");
 
   mManager.OnScrollEnd();
   EXPECT_EQ(FirstCaretAppearance(), Appearance::NormalNotShown);
   check.Call("scrollend1");
 
   mManager.OnScrollStart();
-  EXPECT_EQ(FirstCaretAppearance(), Appearance::None);
+  EXPECT_EQ(FirstCaretAppearance(), Appearance::NormalNotShown);
   check.Call("scrollstart2");
 
   mManager.OnScrollEnd();
   EXPECT_EQ(FirstCaretAppearance(), Appearance::Normal);
   check.Call("scrollend2");
 }
 
 TEST_F(AccessibleCaretManagerTester, TestScrollInCursorModeWhenHidden)
 MOZ_CAN_RUN_SCRIPT
 {
-  // Simulate caret hiding when scrolling.
-  AutoRestore<bool> savesCaretsAlwaysShowWhenScrolling(
-    MockAccessibleCaretManager::sCaretsAlwaysShowWhenScrolling);
-  MockAccessibleCaretManager::sCaretsAlwaysShowWhenScrolling = false;
-
   EXPECT_CALL(mManager, GetCaretMode())
     .WillRepeatedly(Return(CaretMode::Cursor));
 
   EXPECT_CALL(mManager, HasNonEmptyTextContent(_))
     .WillRepeatedly(Return(true));
 
   MockFunction<void(std::string aCheckPointName)> check;
   {
@@ -662,55 +656,50 @@ MOZ_CAN_RUN_SCRIPT
   mManager.OnScrollEnd();
   EXPECT_EQ(FirstCaretAppearance(), Appearance::None);
   check.Call("scrollend2");
 }
 
 TEST_F(AccessibleCaretManagerTester, TestScrollInCursorModeOnEmptyContent)
 MOZ_CAN_RUN_SCRIPT
 {
-  // Simulate caret hiding when scrolling.
-  AutoRestore<bool> savesCaretsAlwaysShowWhenScrolling(
-    MockAccessibleCaretManager::sCaretsAlwaysShowWhenScrolling);
-  MockAccessibleCaretManager::sCaretsAlwaysShowWhenScrolling = false;
-
   EXPECT_CALL(mManager, GetCaretMode())
     .WillRepeatedly(Return(CaretMode::Cursor));
 
   EXPECT_CALL(mManager, HasNonEmptyTextContent(_))
     .WillRepeatedly(Return(false));
 
   MockFunction<void(std::string aCheckPointName)> check;
   {
     InSequence dummy;
 
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
                    CaretChangedReason::Updateposition));
     EXPECT_CALL(check, Call("updatecarets"));
 
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
-                   CaretChangedReason::Visibilitychange));
+                   CaretChangedReason::Scroll));
     EXPECT_CALL(check, Call("scrollstart1"));
 
     EXPECT_CALL(mManager.FirstCaret(), SetPosition(_, _))
       .WillOnce(Return(PositionChangedResult::Invisible));
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
                    CaretChangedReason::Updateposition));
     EXPECT_CALL(check, Call("scrollend1"));
 
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
-                   CaretChangedReason::Visibilitychange));
+                   CaretChangedReason::Scroll));
     EXPECT_CALL(check, Call("scrollstart2"));
 
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
                    CaretChangedReason::Updateposition));
     EXPECT_CALL(check, Call("scrollend2"));
 
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
-                   CaretChangedReason::Visibilitychange));
+                   CaretChangedReason::Scroll));
     EXPECT_CALL(check, Call("scrollstart3"));
 
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
                    CaretChangedReason::Updateposition));
     EXPECT_CALL(check, Call("scrollend3"));
   }
 
   // Simulate a single tap on an empty content.
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -5563,20 +5563,16 @@ pref("layout.accessiblecaret.caret_shown
 
 // Simulate long tap to select words on the platforms where APZ is not enabled
 // or long tap events does not fired by APZ.
 pref("layout.accessiblecaret.use_long_tap_injector", false);
 
 // By default, carets become tilt only when they are overlapping.
 pref("layout.accessiblecaret.always_tilt", false);
 
-// By default, carets always show when scrolling (either panning for zooming)
-// the page.
-pref("layout.accessiblecaret.always_show_when_scrolling", true);
-
 // 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);