Bug 1474360 - Don't immediately hide selection toolbar after showing it; r?esawin draft
authorJim Chen <nchen@mozilla.com>
Thu, 19 Jul 2018 12:09:16 -0400
changeset 820444 274d503a05c0fb110098aab3c524e165f125e4fb
parent 819215 547144f5596c1a146b208d68d93950a6313080ca
push id116832
push userbmo:nchen@mozilla.com
push dateThu, 19 Jul 2018 16:10:46 +0000
reviewersesawin
bugs1474360
milestone63.0a1
Bug 1474360 - Don't immediately hide selection toolbar after showing it; r?esawin To conform to Android behavior, we intentionally suppress the selection toolbar when an input is first focused. However, that code has a bug where right after we show the selection toolbar, we still think the toolbar should be suppressed, and therefore end up hiding the toolbar immediately. The patch fixes that bug and introduces an ACTION_HIDE action, which is necessary to re-hide the toolbar when the user dismisses it manually. MozReview-Commit-ID: DbLd2MCxSyL
mobile/android/chrome/geckoview/GeckoViewSelectionActionContent.js
mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/SelectionActionDelegateTest.kt
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/BasicSelectionActionDelegate.java
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java
--- a/mobile/android/chrome/geckoview/GeckoViewSelectionActionContent.js
+++ b/mobile/android/chrome/geckoview/GeckoViewSelectionActionContent.js
@@ -16,16 +16,20 @@ class GeckoViewSelectionActionContent ex
   constructor(aModuleName, aMessageManager) {
     super(aModuleName, aMessageManager);
 
     this._seqNo = 0;
     this._isActive = false;
     this._previousMessage = "";
 
     this._actions = [{
+      id: "org.mozilla.geckoview.HIDE",
+      predicate: _ => true,
+      perform: _ => this.handleEvent({type: "pagehide"}),
+    }, {
       id: "org.mozilla.geckoview.CUT",
       predicate: e => !e.collapsed && e.selectionEditable && !this._isPasswordField(e),
       perform: _ => docShell.doCommand("cmd_cut"),
     }, {
       id: "org.mozilla.geckoview.COPY",
       predicate: e => !e.collapsed && !this._isPasswordField(e),
       perform: _ => docShell.doCommand("cmd_copy"),
     }, {
@@ -125,17 +129,18 @@ class GeckoViewSelectionActionContent ex
     let reason = aEvent.reason;
 
     if (this._isActive && !aEvent.caretVisible) {
       // For mozcaretstatechanged, "visibilitychange" means the caret is hidden.
       reason = "visibilitychange";
     } else if (!aEvent.collapsed &&
                !aEvent.selectionVisible) {
       reason = "invisibleselection";
-    } else if (aEvent.selectionEditable &&
+    } else if (!this._isActive &&
+               aEvent.selectionEditable &&
                aEvent.collapsed &&
                reason !== "longpressonemptycontent" &&
                reason !== "taponcaret" &&
                !Services.prefs.getBoolPref(
                    "geckoview.selection_action.show_on_focus", false)) {
       // Don't show selection actions when merely focusing on an editor or
       // repositioning the cursor. Wait until long press or the caret is tapped
       // in order to match Android behavior.
--- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/SelectionActionDelegateTest.kt
+++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/SelectionActionDelegateTest.kt
@@ -73,39 +73,43 @@ class SelectionActionDelegateTest : Base
     /** Generic tests for each content type. */
 
     @Test fun request() {
         if (editable) {
             withClipboard ("text") {
                 testThat(selectedContent, {}, hasShowActionRequest(
                         FLAG_IS_EDITABLE, arrayOf(ACTION_COLLAPSE_TO_START, ACTION_COLLAPSE_TO_END,
                                                   ACTION_COPY, ACTION_CUT, ACTION_DELETE,
-                                                  ACTION_PASTE, ACTION_SELECT_ALL)))
+                                                  ACTION_HIDE, ACTION_PASTE, ACTION_SELECT_ALL)))
             }
         } else {
             testThat(selectedContent, {}, hasShowActionRequest(
-                    0, arrayOf(ACTION_COPY, ACTION_SELECT_ALL, ACTION_UNSELECT)))
+                    0, arrayOf(ACTION_COPY, ACTION_HIDE, ACTION_SELECT_ALL,
+                                           ACTION_UNSELECT)))
         }
     }
 
     @Test fun request_collapsed() = assumingEditable(true) {
         withClipboard ("text") {
             testThat(collapsedContent, {}, hasShowActionRequest(
                     FLAG_IS_EDITABLE or FLAG_IS_COLLAPSED,
-                    arrayOf(ACTION_PASTE, ACTION_SELECT_ALL)))
+                    arrayOf(ACTION_HIDE, ACTION_PASTE, ACTION_SELECT_ALL)))
         }
     }
 
     @Test fun request_noClipboard() = assumingEditable(true) {
         withClipboard("") {
             testThat(collapsedContent, {}, hasShowActionRequest(
-                    FLAG_IS_EDITABLE or FLAG_IS_COLLAPSED, arrayOf(ACTION_SELECT_ALL)))
+                    FLAG_IS_EDITABLE or FLAG_IS_COLLAPSED,
+                    arrayOf(ACTION_HIDE, ACTION_SELECT_ALL)))
         }
     }
 
+    @Test fun hide() = testThat(selectedContent, withResponse(ACTION_HIDE), clearsSelection())
+
     @Test fun cut() = assumingEditable(true) {
         withClipboard("") {
             testThat(selectedContent, withResponse(ACTION_CUT), copiesText(), deletesContent())
         }
     }
 
     @Test fun copy() = withClipboard("") {
         testThat(selectedContent, withResponse(ACTION_COPY), copiesText())
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/BasicSelectionActionDelegate.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/BasicSelectionActionDelegate.java
@@ -219,18 +219,20 @@ public class BasicSelectionActionDelegat
 
     /**
      * Clear the current selection, if possible.
      */
     protected void clearSelection() {
         if (mResponse != null) {
             if (isActionAvailable(ACTION_COLLAPSE_TO_END)) {
                 mResponse.respond(ACTION_COLLAPSE_TO_END);
+            } else if (isActionAvailable(ACTION_UNSELECT)) {
+                mResponse.respond(ACTION_UNSELECT);
             } else {
-                mResponse.respond(ACTION_UNSELECT);
+                mResponse.respond(ACTION_HIDE);
             }
         }
     }
 
     private Intent getProcessTextIntent() {
         final Intent intent = new Intent(Intent.ACTION_PROCESS_TEXT);
         intent.addCategory(Intent.CATEGORY_DEFAULT);
         intent.setType("text/plain");
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java
@@ -2037,27 +2037,32 @@ public class GeckoSession extends LayerS
          * contentEditable node.
          */
         final int FLAG_IS_EDITABLE = 2;
         /**
          * The selection is inside a password field.
          */
         final int FLAG_IS_PASSWORD = 4;
 
-        @StringDef({ACTION_CUT,
+        @StringDef({ACTION_HIDE,
+                    ACTION_CUT,
                     ACTION_COPY,
                     ACTION_DELETE,
                     ACTION_PASTE,
                     ACTION_SELECT_ALL,
                     ACTION_UNSELECT,
                     ACTION_COLLAPSE_TO_START,
                     ACTION_COLLAPSE_TO_END})
         /* package */ @interface Action {}
 
         /**
+         * Hide selection actions and cause {@link #onHideAction} to be called.
+         */
+        final String ACTION_HIDE = "org.mozilla.geckoview.HIDE";
+        /**
          * Copy onto the clipboard then delete the selected content. Selection
          * must be editable.
          */
         final String ACTION_CUT = "org.mozilla.geckoview.CUT";
         /**
          * Copy the selected content onto the clipboard.
          */
         final String ACTION_COPY = "org.mozilla.geckoview.COPY";
@@ -2146,17 +2151,17 @@ public class GeckoSession extends LayerS
          *
          * Once a {@link #onHideAction} call (with particular reasons) or another {@link
          * #onShowActionRequest} call is received, any previously received actions are no
          * longer unavailable.
          *
          * @param session The GeckoSession that initiated the callback.
          * @param selection Current selection attributes.
          * @param actions Array of built-in actions available; possible values
-         * come from the {@link #ACTION_CUT ACTION_*} constants.
+         * come from the {@link #ACTION_HIDE ACTION_*} constants.
          * @param response Callback object for performing built-in actions. For example,
          * {@code response.respond(actions[0])} performs the first action. May be used
          * multiple times to perform multiple actions at once.
          */
         void onShowActionRequest(GeckoSession session, Selection selection,
                                  @Action String[] actions, GeckoResponse<String> response);
 
         @IntDef({HIDE_REASON_NO_SELECTION,
@@ -2165,19 +2170,18 @@ public class GeckoSession extends LayerS
                  HIDE_REASON_ACTIVE_SCROLL})
         /* package */ @interface HideReason {}
 
         /**
          * Actions are no longer available due to the user clearing the selection.
          */
         final int HIDE_REASON_NO_SELECTION = 0;
         /**
-         * Actions are no longer available due to the user moving the selection becoming
-         * out of view. Previous actions are still available after a callback with this
-         * reason.
+         * Actions are no longer available due to the user moving the selection out of view.
+         * Previous actions are still available after a callback with this reason.
          */
         final int HIDE_REASON_INVISIBLE_SELECTION = 1;
         /**
          * Actions are no longer available due to the user actively changing the
          * selection. {@link #onShowActionRequest} may be called again once the user has
          * set a selection, if the new selection has available actions.
          */
         final int HIDE_REASON_ACTIVE_SELECTION = 2;