Bug 1445794 Part 1 - Preemptively fix that carets are not updated if non-default hints are used. r=emilio
authorTing-Yu Lin <aethanyc@gmail.com>
Thu, 03 Jan 2019 05:25:19 +0000
changeset 509573 34151b0c56e9cdd7c8df1b613fca55900f3274e1
parent 509572 4d42e4786e4fef58c9fd4e26f0f35dcc9cdf3df3
child 509574 267a452439f57efafc138d7a03effa73c23adf0d
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1445794
milestone66.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 1445794 Part 1 - Preemptively fix that carets are not updated if non-default hints are used. r=emilio Without this change, for example, UpdateCarets(UpdateCaretsHint::DispatchNoEvent) won't update carets. We don't have a wrong use case yet, but it's good to fix that beforehand. Differential Revision: https://phabricator.services.mozilla.com/D15535
layout/base/AccessibleCaretManager.cpp
--- a/layout/base/AccessibleCaretManager.cpp
+++ b/layout/base/AccessibleCaretManager.cpp
@@ -239,17 +239,17 @@ void AccessibleCaretManager::UpdateCaret
     return;
   }
 
   PositionChangedResult result = mFirstCaret->SetPosition(frame, offset);
 
   switch (result) {
     case PositionChangedResult::NotChanged:
     case PositionChangedResult::Changed:
-      if (aHints == UpdateCaretsHint::Default) {
+      if (!aHints.contains(UpdateCaretsHint::RespectOldAppearance)) {
         if (HasNonEmptyTextContent(GetEditingHostForFrame(frame))) {
           mFirstCaret->SetAppearance(Appearance::Normal);
         } else if (
             StaticPrefs::
                 layout_accessiblecaret_caret_shown_when_long_tapping_on_empty_content()) {
           if (mFirstCaret->IsLogicallyVisible()) {
             // Possible cases are: 1) SelectWordOrShortcut() sets the
             // appearance to Normal. 2) When the caret is out of viewport and
@@ -263,19 +263,16 @@ void AccessibleCaretManager::UpdateCaret
             //
             // Do nothing to make the appearance remains None so that it can
             // be distinguished from case 2). Also do not set the appearance
             // to NormalNotShown here like the default update behavior.
           }
         } else {
           mFirstCaret->SetAppearance(Appearance::NormalNotShown);
         }
-      } else if (aHints.contains(UpdateCaretsHint::RespectOldAppearance)) {
-        // Do nothing to preserve the appearance of the caret set by the
-        // caller.
       }
       break;
 
     case PositionChangedResult::Invisible:
       mFirstCaret->SetAppearance(Appearance::NormalNotShown);
       break;
   }
 
@@ -306,21 +303,18 @@ void AccessibleCaretManager::UpdateCaret
 
   auto updateSingleCaret = [aHints](AccessibleCaret* aCaret, nsIFrame* aFrame,
                                     int32_t aOffset) -> PositionChangedResult {
     PositionChangedResult result = aCaret->SetPosition(aFrame, aOffset);
 
     switch (result) {
       case PositionChangedResult::NotChanged:
       case PositionChangedResult::Changed:
-        if (aHints == UpdateCaretsHint::Default) {
+        if (!aHints.contains(UpdateCaretsHint::RespectOldAppearance)) {
           aCaret->SetAppearance(Appearance::Normal);
-        } else if (aHints.contains(UpdateCaretsHint::RespectOldAppearance)) {
-          // Do nothing to preserve the appearance of the caret set by the
-          // caller.
         }
         break;
 
       case PositionChangedResult::Invisible:
         aCaret->SetAppearance(Appearance::NormalNotShown);
         break;
     }
     return result;
@@ -334,19 +328,20 @@ void AccessibleCaretManager::UpdateCaret
   if (firstCaretResult == PositionChangedResult::Changed ||
       secondCaretResult == PositionChangedResult::Changed) {
     // Flush layout to make the carets intersection correct.
     if (!FlushLayout()) {
       return;
     }
   }
 
-  if (aHints == UpdateCaretsHint::Default) {
-    // Only check for tilt carets with default update hint. Otherwise we might
-    // override the appearance set by the caller.
+  if (!aHints.contains(UpdateCaretsHint::RespectOldAppearance)) {
+    // Only check for tilt carets when the caller doesn't ask us to preserve
+    // old appearance. Otherwise we might override the appearance set by the
+    // caller.
     if (StaticPrefs::layout_accessiblecaret_always_tilt()) {
       UpdateCaretsForAlwaysTilt(startFrame, endFrame);
     } else {
       UpdateCaretsForOverlappingTilt();
     }
   }
 
   if (!aHints.contains(UpdateCaretsHint::DispatchNoEvent) && !mActiveCaret) {