From bcbf8175b89c594637fc66fb1c38e658b4b648ee Mon Sep 17 00:00:00 2001 From: Ting-Yu Lin Date: Thu, 21 Apr 2016 16:53:40 +0800 Subject: [PATCH] Bug 1249201 Part 2 - Show carets continuously when panning or zooming. r=mats,sebastian Add a pref "layout.accessiblecaret.always_show_when_scrolling" defaults to true on all platforms except b2g. When it is set to false, the carets will be hidden during scrolling, which is the current behavior before applying this change. The pref "layout.accessiblecaret.extendedvisibility" was added for Fennec to keep ActionBar open when carets temporarily hiding during panning or zooming. Now we make carets always show by default, so the pref can be removed. However, the floating toolbar still need to be notified when the scrolling begins, so we dispatch "scroll" instead. In gtest, the preference changes were in the middle of the test function. To make the preference change clearer, I add new pref changes or move the existing ones to the beginning of the test functions. The 250ms transition effect added in ua.css is per request of UX designer in bug 1249201 comment 12. MozReview-Commit-ID: 8NGvDLPbtNY --HG-- extra : rebase_source : 3f7a9ebdf4c70b0282dbf9e8f18cbe5cca656dbe --- b2g/app/b2g.js | 3 + layout/base/AccessibleCaretManager.cpp | 46 ++++++------- layout/base/AccessibleCaretManager.h | 13 ++-- .../base/gtest/TestAccessibleCaretManager.cpp | 67 ++++++++++++------- layout/style/res/ua.css | 6 ++ mobile/android/app/mobile.js | 5 -- .../chrome/content/ActionBarHandler.js | 3 +- modules/libpref/init/all.js | 7 +- 8 files changed, 83 insertions(+), 67 deletions(-) diff --git a/b2g/app/b2g.js b/b2g/app/b2g.js index f626d1535dbc..70a298f70a6d 100644 --- a/b2g/app/b2g.js +++ b/b2g/app/b2g.js @@ -1020,6 +1020,9 @@ pref("layout.accessiblecaret.bar.enabled", true); pref("layout.accessiblecaret.use_long_tap_injector", false); #endif +// Hide carets and text selection dialog during scrolling. +pref("layout.accessiblecaret.always_show_when_scrolling", false); + // Enable sync and mozId with Firefox Accounts. pref("services.sync.fxaccounts.enabled", true); pref("identity.fxaccounts.enabled", true); diff --git a/layout/base/AccessibleCaretManager.cpp b/layout/base/AccessibleCaretManager.cpp index 3bea78545ff7..bde0f5ca4608 100644 --- a/layout/base/AccessibleCaretManager.cpp +++ b/layout/base/AccessibleCaretManager.cpp @@ -72,10 +72,10 @@ AccessibleCaretManager::sSelectionBarEnabled = false; /*static*/ bool AccessibleCaretManager::sCaretShownWhenLongTappingOnEmptyContent = false; /*static*/ bool -AccessibleCaretManager::sCaretsExtendedVisibility = false; -/*static*/ bool AccessibleCaretManager::sCaretsAlwaysTilt = false; /*static*/ bool +AccessibleCaretManager::sCaretsAlwaysShowWhenScrolling = true; +/*static*/ bool AccessibleCaretManager::sCaretsScriptUpdates = false; /*static*/ bool AccessibleCaretManager::sCaretsAllowDraggingAcrossOtherCaret = true; @@ -102,10 +102,10 @@ AccessibleCaretManager::AccessibleCaretManager(nsIPresShell* aPresShell) "layout.accessiblecaret.bar.enabled"); 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(&sCaretsAlwaysShowWhenScrolling, + "layout.accessiblecaret.always_show_when_scrolling", true); Preferences::AddBoolVarCache(&sCaretsScriptUpdates, "layout.accessiblecaret.allow_script_change_updates"); Preferences::AddBoolVarCache(&sCaretsAllowDraggingAcrossOtherCaret, @@ -201,18 +201,6 @@ AccessibleCaretManager::HideCarets() } } -void -AccessibleCaretManager::DoNotShowCarets() -{ - if (mFirstCaret->IsLogicallyVisible() || mSecondCaret->IsLogicallyVisible()) { - AC_LOG("%s", __FUNCTION__); - mFirstCaret->SetAppearance(Appearance::NormalNotShown); - mSecondCaret->SetAppearance(Appearance::NormalNotShown); - DispatchCaretStateChangedEvent(CaretChangedReason::Visibilitychange); - CancelCaretTimeoutTimer(); - } -} - void AccessibleCaretManager::UpdateCarets(UpdateCaretsHint aHint) { @@ -626,14 +614,19 @@ AccessibleCaretManager::OnScrollStart() { AC_LOG("%s", __FUNCTION__); - mFirstCaretAppearanceOnScrollStart = mFirstCaret->GetAppearance(); - mSecondCaretAppearanceOnScrollStart = mSecondCaret->GetAppearance(); - - // Hide the carets. (Extended visibility makes them "NormalNotShown"). - if (sCaretsExtendedVisibility) { - DoNotShowCarets(); - } else { + 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); } } @@ -644,8 +637,11 @@ AccessibleCaretManager::OnScrollEnd() return; } - mFirstCaret->SetAppearance(mFirstCaretAppearanceOnScrollStart); - mSecondCaret->SetAppearance(mSecondCaretAppearanceOnScrollStart); + 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()) { diff --git a/layout/base/AccessibleCaretManager.h b/layout/base/AccessibleCaretManager.h index 617936be4567..8a05c7295976 100644 --- a/layout/base/AccessibleCaretManager.h +++ b/layout/base/AccessibleCaretManager.h @@ -135,10 +135,6 @@ protected: // Force hiding all carets regardless of the current selection status. void HideCarets(); - // Force carets to be "present" logically, but not visible. Allows ActionBar - // to stay open when carets visibility is supressed during scroll. - void DoNotShowCarets(); - void UpdateCaretsForCursorMode(UpdateCaretsHint aHint); void UpdateCaretsForSelectionMode(UpdateCaretsHint aHint); @@ -298,14 +294,15 @@ protected: // heuristic. See UpdateCaretsForCursorMode() for the details. static bool sCaretShownWhenLongTappingOnEmptyContent; - // Android specific visibility extensions correct compatibility issues - // with ActionBar visibility during page scroll. - static bool sCaretsExtendedVisibility; - // 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. diff --git a/layout/base/gtest/TestAccessibleCaretManager.cpp b/layout/base/gtest/TestAccessibleCaretManager.cpp index 3a90a5d5c476..78ec6eea9971 100644 --- a/layout/base/gtest/TestAccessibleCaretManager.cpp +++ b/layout/base/gtest/TestAccessibleCaretManager.cpp @@ -60,8 +60,8 @@ public: using AccessibleCaretManager::UpdateCarets; using AccessibleCaretManager::HideCarets; using AccessibleCaretManager::sCaretShownWhenLongTappingOnEmptyContent; - using AccessibleCaretManager::sCaretsExtendedVisibility; using AccessibleCaretManager::sCaretsAlwaysTilt; + using AccessibleCaretManager::sCaretsAlwaysShowWhenScrolling; MockAccessibleCaretManager() : AccessibleCaretManager(nullptr) @@ -350,6 +350,11 @@ TEST_F(AccessibleCaretManagerTester, TestTypingAtEndOfInput) TEST_F(AccessibleCaretManagerTester, TestScrollInSelectionMode) { + // Simulate B2G preference. + AutoRestore savesCaretsAlwaysShowWhenScrolling( + MockAccessibleCaretManager::sCaretsAlwaysShowWhenScrolling); + MockAccessibleCaretManager::sCaretsAlwaysShowWhenScrolling = false; + EXPECT_CALL(mManager, GetCaretMode()) .WillRepeatedly(Return(CaretMode::Selection)); @@ -422,9 +427,13 @@ TEST_F(AccessibleCaretManagerTester, TestScrollInSelectionMode) check.Call("scrollend2"); } -TEST_F(AccessibleCaretManagerTester, - TestScrollInSelectionModeWithExtendedVisibilityAndAlwaysTilt) +TEST_F(AccessibleCaretManagerTester, TestScrollInSelectionModeWithAlwaysTiltPref) { + // Simulate Firefox Android preference. + AutoRestore saveCaretsAlwaysTilt( + MockAccessibleCaretManager::sCaretsAlwaysTilt); + MockAccessibleCaretManager::sCaretsAlwaysTilt = true; + EXPECT_CALL(mManager, GetCaretMode()) .WillRepeatedly(Return(CaretMode::Selection)); @@ -441,7 +450,7 @@ TEST_F(AccessibleCaretManagerTester, EXPECT_CALL(check, Call("updatecarets")); EXPECT_CALL(mManager, DispatchCaretStateChangedEvent( - CaretChangedReason::Visibilitychange)); + CaretChangedReason::Scroll)); EXPECT_CALL(check, Call("scrollstart1")); EXPECT_CALL(mManager, DispatchCaretStateChangedEvent( @@ -458,7 +467,7 @@ TEST_F(AccessibleCaretManagerTester, EXPECT_CALL(check, Call("scrollend1")); EXPECT_CALL(mManager, DispatchCaretStateChangedEvent( - CaretChangedReason::Visibilitychange)); + CaretChangedReason::Scroll)); EXPECT_CALL(check, Call("scrollstart2")); EXPECT_CALL(mManager, DispatchCaretStateChangedEvent( @@ -471,14 +480,6 @@ TEST_F(AccessibleCaretManagerTester, EXPECT_CALL(check, Call("scrollend2")); } - // Simulate Firefox Android preferences. - AutoRestore saveCaretsExtendedVisibility( - MockAccessibleCaretManager::sCaretsExtendedVisibility); - MockAccessibleCaretManager::sCaretsExtendedVisibility = true; - AutoRestore saveCaretsAlwaysTilt( - MockAccessibleCaretManager::sCaretsAlwaysTilt); - MockAccessibleCaretManager::sCaretsAlwaysTilt = true; - mManager.UpdateCarets(); EXPECT_EQ(FirstCaretAppearance(), Appearance::NormalNotShown); EXPECT_EQ(SecondCaretAppearance(), Appearance::Right); @@ -486,12 +487,12 @@ TEST_F(AccessibleCaretManagerTester, mManager.OnScrollStart(); EXPECT_EQ(FirstCaretAppearance(), Appearance::NormalNotShown); - EXPECT_EQ(SecondCaretAppearance(), Appearance::NormalNotShown); + EXPECT_EQ(SecondCaretAppearance(), Appearance::Right); check.Call("scrollstart1"); mManager.OnReflow(); EXPECT_EQ(FirstCaretAppearance(), Appearance::NormalNotShown); - EXPECT_EQ(SecondCaretAppearance(), Appearance::NormalNotShown); + EXPECT_EQ(SecondCaretAppearance(), Appearance::Right); check.Call("reflow1"); mManager.OnScrollEnd(); @@ -500,12 +501,12 @@ TEST_F(AccessibleCaretManagerTester, check.Call("scrollend1"); mManager.OnScrollStart(); - EXPECT_EQ(FirstCaretAppearance(), Appearance::NormalNotShown); + EXPECT_EQ(FirstCaretAppearance(), Appearance::Left); EXPECT_EQ(SecondCaretAppearance(), Appearance::NormalNotShown); check.Call("scrollstart2"); mManager.OnReflow(); - EXPECT_EQ(FirstCaretAppearance(), Appearance::NormalNotShown); + EXPECT_EQ(FirstCaretAppearance(), Appearance::Left); EXPECT_EQ(SecondCaretAppearance(), Appearance::NormalNotShown); check.Call("reflow2"); @@ -517,6 +518,11 @@ TEST_F(AccessibleCaretManagerTester, TEST_F(AccessibleCaretManagerTester, TestScrollInCursorModeWhenLogicallyVisible) { + // Simulate B2G preference. + AutoRestore savesCaretsAlwaysShowWhenScrolling( + MockAccessibleCaretManager::sCaretsAlwaysShowWhenScrolling); + MockAccessibleCaretManager::sCaretsAlwaysShowWhenScrolling = false; + EXPECT_CALL(mManager, GetCaretMode()) .WillRepeatedly(Return(CaretMode::Cursor)); @@ -577,6 +583,11 @@ TEST_F(AccessibleCaretManagerTester, TestScrollInCursorModeWhenLogicallyVisible) TEST_F(AccessibleCaretManagerTester, TestScrollInCursorModeWhenHidden) { + // Simulate B2G preference. + AutoRestore savesCaretsAlwaysShowWhenScrolling( + MockAccessibleCaretManager::sCaretsAlwaysShowWhenScrolling); + MockAccessibleCaretManager::sCaretsAlwaysShowWhenScrolling = false; + EXPECT_CALL(mManager, GetCaretMode()) .WillRepeatedly(Return(CaretMode::Cursor)); @@ -631,6 +642,11 @@ TEST_F(AccessibleCaretManagerTester, TestScrollInCursorModeWhenHidden) TEST_F(AccessibleCaretManagerTester, TestScrollInCursorModeOnEmptyContent) { + // Simulate B2G preference. + AutoRestore savesCaretsAlwaysShowWhenScrolling( + MockAccessibleCaretManager::sCaretsAlwaysShowWhenScrolling); + MockAccessibleCaretManager::sCaretsAlwaysShowWhenScrolling = false; + EXPECT_CALL(mManager, GetCaretMode()) .WillRepeatedly(Return(CaretMode::Cursor)); @@ -700,8 +716,13 @@ TEST_F(AccessibleCaretManagerTester, TestScrollInCursorModeOnEmptyContent) } TEST_F(AccessibleCaretManagerTester, - TestScrollInCursorModeOnEmptyContentWithSpecialPreference) + TestScrollInCursorModeWithCaretShownWhenLongTappingOnEmptyContentPref) { + // Simulate Firefox Android preference. + AutoRestore savesCaretShownWhenLongTappingOnEmptyContent( + MockAccessibleCaretManager::sCaretShownWhenLongTappingOnEmptyContent); + MockAccessibleCaretManager::sCaretShownWhenLongTappingOnEmptyContent = true; + EXPECT_CALL(mManager, GetCaretMode()) .WillRepeatedly(Return(CaretMode::Cursor)); @@ -721,7 +742,7 @@ TEST_F(AccessibleCaretManagerTester, EXPECT_CALL(check, Call("longtap updatecarets")); EXPECT_CALL(mManager, DispatchCaretStateChangedEvent( - CaretChangedReason::Visibilitychange)); + CaretChangedReason::Scroll)); EXPECT_CALL(check, Call("longtap scrollstart1")); EXPECT_CALL(mManager.FirstCaret(), SetPosition(_, _)) @@ -731,7 +752,7 @@ TEST_F(AccessibleCaretManagerTester, EXPECT_CALL(check, Call("longtap scrollend1")); EXPECT_CALL(mManager, DispatchCaretStateChangedEvent( - CaretChangedReason::Visibilitychange)); + CaretChangedReason::Scroll)); EXPECT_CALL(check, Call("longtap scrollstart2")); EXPECT_CALL(mManager, DispatchCaretStateChangedEvent( @@ -739,7 +760,7 @@ TEST_F(AccessibleCaretManagerTester, EXPECT_CALL(check, Call("longtap scrollend2")); EXPECT_CALL(mManager, DispatchCaretStateChangedEvent( - CaretChangedReason::Visibilitychange)); + CaretChangedReason::Scroll)); EXPECT_CALL(check, Call("longtap scrollstart3")); EXPECT_CALL(mManager, DispatchCaretStateChangedEvent( @@ -747,10 +768,6 @@ TEST_F(AccessibleCaretManagerTester, EXPECT_CALL(check, Call("longtap scrollend3")); } - AutoRestore savePref( - MockAccessibleCaretManager::sCaretShownWhenLongTappingOnEmptyContent); - MockAccessibleCaretManager::sCaretShownWhenLongTappingOnEmptyContent = true; - // Simulate a single tap on an empty input. mManager.FirstCaret().SetAppearance(Appearance::None); mManager.UpdateCarets(); diff --git a/layout/style/res/ua.css b/layout/style/res/ua.css index 1c22076a208f..7c3c8f8ab3c1 100644 --- a/layout/style/res/ua.css +++ b/layout/style/res/ua.css @@ -342,6 +342,12 @@ parsererror|sourcetext { font-size: 12pt; } +div:-moz-native-anonymous.moz-accessiblecaret { + /* Add transition effect to make caret size changing smoother. */ + transition-duration: 250ms; + transition-property: width, height, margin-left; +} + div:-moz-native-anonymous.moz-accessiblecaret, div:-moz-native-anonymous.moz-accessiblecaret > div.image, div:-moz-native-anonymous.moz-accessiblecaret > div.bar { diff --git a/mobile/android/app/mobile.js b/mobile/android/app/mobile.js index 1ba76dad63a9..c1b556224f0f 100644 --- a/mobile/android/app/mobile.js +++ b/mobile/android/app/mobile.js @@ -889,11 +889,6 @@ pref("layout.accessiblecaret.timeout_ms", 0); // Android generates long tap (mouse) events. pref("layout.accessiblecaret.use_long_tap_injector", false); -// AccessibleCarets behaviour is extended to support Android specific -// requirements to hide carets while maintaining ActionBar visiblity during page -// scroll. -pref("layout.accessiblecaret.extendedvisibility", true); - // Androids carets are always tilt to match the text selection guideline. pref("layout.accessiblecaret.always_tilt", true); diff --git a/mobile/android/chrome/content/ActionBarHandler.js b/mobile/android/chrome/content/ActionBarHandler.js index f8f19c6bbd76..fc3a09b17187 100644 --- a/mobile/android/chrome/content/ActionBarHandler.js +++ b/mobile/android/chrome/content/ActionBarHandler.js @@ -65,7 +65,8 @@ var ActionBarHandler = { if (this._selectionID) { let [element, win] = this._getSelectionTargets(); if (this._targetElement === element && this._contentWindow === win) { - if (e.reason == 'visibilitychange' || e.reason == 'presscaret') { + if (e.reason == 'visibilitychange' || e.reason == 'presscaret' || + e.reason == 'scroll' ) { this._updateVisibility(); } else { let forceUpdate = e.reason == 'updateposition' || e.reason == 'releasecaret'; diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index a55b4a107e38..420eaaa7c8e4 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -5064,12 +5064,13 @@ pref("layout.accessiblecaret.timeout_ms", 3000); // or long tap events does not fired by APZ. pref("layout.accessiblecaret.use_long_tap_injector", true); -// Use AccessibleCaret default behaviours. -pref("layout.accessiblecaret.extendedvisibility", 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);