Bug 853691 - Reorganize SelectionHandler cleanup code paths. r=bnicholson
authorMargaret Leibovic <margaret.leibovic@gmail.com>
Mon, 25 Mar 2013 18:25:19 -0400
changeset 136580 c8d7e2709f01c8a746514f3abb66e78a33989845
parent 136579 cb015c10d58f1306e56cb76564b0c4793790f263
child 136581 3a535bd50a23339d99808ac0e312f7ccdee2b48b
push id2452
push userlsblakk@mozilla.com
push dateMon, 13 May 2013 16:59:38 +0000
treeherdermozilla-beta@d4b152d29d8d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbnicholson
bugs853691
milestone22.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 853691 - Reorganize SelectionHandler cleanup code paths. r=bnicholson
mobile/android/base/TextSelection.java
mobile/android/chrome/content/SelectionHandler.js
mobile/android/chrome/content/browser.js
--- a/mobile/android/base/TextSelection.java
+++ b/mobile/android/base/TextSelection.java
@@ -84,26 +84,24 @@ class TextSelection extends Layer implem
                         mViewLeft = 0.0f;
                         mViewTop = 0.0f;
                         mViewZoom = 0.0f;
                         LayerView layerView = mActivity.getLayerView();
                         if (layerView != null) {
                             layerView.addLayer(TextSelection.this);
                         }
                     } else if (event.equals("TextSelection:HideHandles")) {
-                        final JSONArray handles = message.getJSONArray("handles");
                         LayerView layerView = mActivity.getLayerView();
                         if (layerView != null) {
                             layerView.removeLayer(TextSelection.this);
                         }
 
-                        for (int i=0; i < handles.length(); i++) {
-                            String handle = handles.getString(i);
-                            getHandle(handle).setVisibility(View.GONE);
-                        }
+                        mStartHandle.setVisibility(View.GONE);
+                        mMiddleHandle.setVisibility(View.GONE);
+                        mEndHandle.setVisibility(View.GONE);
                     } else if (event.equals("TextSelection:PositionHandles")) {
                         final boolean rtl = message.getBoolean("rtl");
                         final JSONArray positions = message.getJSONArray("positions");
                         for (int i=0; i < positions.length(); i++) {
                             JSONObject position = positions.getJSONObject(i);
                             int left = position.getInt("left");
                             int top = position.getInt("top");
 
--- a/mobile/android/chrome/content/SelectionHandler.js
+++ b/mobile/android/chrome/content/SelectionHandler.js
@@ -66,30 +66,32 @@ var SelectionHandler = {
     BrowserApp.deck.removeEventListener("compositionend", this);
   },
 
   observe: function sh_observe(aSubject, aTopic, aData) {
     switch (aTopic) {
       case "Gesture:SingleTap": {
         if (this._activeType == this.TYPE_SELECTION) {
           let data = JSON.parse(aData);
-          this.endSelection(data.x, data.y);
+          if (this._pointInSelection(data.x, data.y))
+            this.copySelection();
+          else
+            this._closeSelection();
         }
         break;
       }
       case "Tab:Selected":
-        if (this._activeType == this.TYPE_CURSOR) {
-          this.hideThumb();
-        }
-        // fall through
+        this._closeSelection();
+        break;
+  
       case "Window:Resize": {
         if (this._activeType == this.TYPE_SELECTION) {
           // Knowing when the page is done drawing is hard, so let's just cancel
           // the selection when the window changes. We should fix this later.
-          this.endSelection();
+          this._closeSelection();
         }
         break;
       }
       case "after-viewport-change": {
         if (this._activeType == this.TYPE_SELECTION) {
           // Update the cache after the viewport changes (e.g. panning, zooming).
           this.updateCacheForSelection();
         }
@@ -133,32 +135,25 @@ var SelectionHandler = {
         break;
       }
     }
   },
 
   handleEvent: function sh_handleEvent(aEvent) {
     switch (aEvent.type) {
       case "pagehide":
-        if (this._activeType == this.TYPE_SELECTION)
-          this.endSelection();
-        else
-          this.hideThumb();
-        break;
-
+      // We only add keydown and blur listeners for TYPE_CURSOR
       case "keydown":
       case "blur":
-        if (this._activeType == this.TYPE_CURSOR)
-          this.hideThumb();
+        this._closeSelection();
         break;
 
       case "compositionend":
-        // If the handles are displayed during user input, hide them.
         if (this._activeType == this.TYPE_CURSOR) {
-          this.hideThumb();
+          this._closeSelection();
         }
         break;
     }
   },
 
   _ignoreCollapsedSelection: false,
 
   notifySelectionChanged: function sh_notifySelectionChanged(aDoc, aSel, aReason) {
@@ -169,18 +164,18 @@ var SelectionHandler = {
 
       // If the selection is collapsed because of one of the mouse events we
       // sent while moving the handle, don't get rid of the selection handles.
       if (aReason & Ci.nsISelectionListener.MOUSEDOWN_REASON) {
         this._ignoreCollapsedSelection = true;
         return;
       }
 
-      // Otherwise, we do want to end the selection.
-      this.endSelection();
+      // Otherwise, we do want to close the selection.
+      this._closeSelection();
     }
 
     this._ignoreCollapsedSelection = false;
   },
 
   /** Returns true if the provided element can be selected in text selection, false otherwise. */
   canSelect: function sh_canSelect(aElement) {
     return !(aElement instanceof Ci.nsIDOMHTMLButtonElement ||
@@ -188,22 +183,17 @@ var SelectionHandler = {
              aElement instanceof Ci.nsIDOMHTMLImageElement ||
              aElement instanceof Ci.nsIDOMHTMLMediaElement) &&
              aElement.style.MozUserSelect != 'none';
   },
 
   // aX/aY are in top-level window browser coordinates
   startSelection: function sh_startSelection(aElement, aX, aY) {
     // Clear out any existing selection
-    if (this._activeType == this.TYPE_SELECTION) {
-      this.endSelection();
-    } else if (this._activeType == this.TYPE_CURSOR) {
-      // Hide the cursor handles.
-      this.hideThumb();
-    }
+    this._closeSelection();
 
     // Get the element's view
     this._view = aElement.ownerDocument.defaultView;
 
     if (aElement instanceof Ci.nsIDOMNSEditableElement)
       this._target = aElement;
     else
       this._target = this._view;
@@ -225,24 +215,24 @@ var SelectionHandler = {
 
       // Select the word nearest the caret
       selectionController.wordMove(false, false);
 
       // Move forward in LTR, backward in RTL
       selectionController.wordMove(!this._isRTL, true);
     } catch(e) {
       // If we couldn't select the word at the given point, bail
-      this._cleanUp();
+      this._closeSelection();
       return;
     }
 
     // If there isn't an appropriate selection, bail
     if (!selection.rangeCount || !selection.getRangeAt(0) || !selection.toString().trim().length) {
       selection.collapseToStart();
-      this._cleanUp();
+      this._closeSelection();
       return;
     }
 
     // Add a listener to end the selection if it's removed programatically
     selection.QueryInterface(Ci.nsISelectionPrivate).addSelectionListener(this);
 
     // Initialize the cache
     this.cache = { start: {}, end: {}};
@@ -262,16 +252,23 @@ var SelectionHandler = {
 
   getSelection: function sh_getSelection() {
     if (this._target instanceof Ci.nsIDOMNSEditableElement)
       return this._target.QueryInterface(Ci.nsIDOMNSEditableElement).editor.selection;
     else
       return this._target.getSelection();
   },
 
+  _getSelectedText: function sh_getSelectedText() {
+    let selection = this.getSelection();
+    if (selection)
+      return selection.toString().trim();
+    return "";
+  },
+
   getSelectionController: function sh_getSelectionController() {
     if (this._target instanceof Ci.nsIDOMNSEditableElement)
       return this._target.QueryInterface(Ci.nsIDOMNSEditableElement).editor.selectionController;
     else
       return this._target.QueryInterface(Ci.nsIInterfaceRequestor).
                           getInterface(Ci.nsIWebNavigation).
                           QueryInterface(Ci.nsIInterfaceRequestor).
                           getInterface(Ci.nsISelectionDisplay).
@@ -365,67 +362,63 @@ var SelectionHandler = {
       // Send mouse event 1px too high to prevent selection from entering the line below where it should be
       aY -= 1;
     }
 
     this._cwu.sendMouseEventToWindow("mousedown", aX, aY, 0, 0, useShift ? Ci.nsIDOMNSEvent.SHIFT_MASK : 0, true);
     this._cwu.sendMouseEventToWindow("mouseup", aX, aY, 0, 0, useShift ? Ci.nsIDOMNSEvent.SHIFT_MASK : 0, true);
   },
 
-  // aX/aY are in top-level window browser coordinates
-  endSelection: function sh_endSelection(aX, aY) {
-    if (this._activeType != this.TYPE_SELECTION)
-      return "";
+  copySelection: function sh_copySelection() {
+    let selectedText = this._getSelectedText();
+    if (selectedText.length) {
+      let clipboard = Cc["@mozilla.org/widget/clipboardhelper;1"].getService(Ci.nsIClipboardHelper);
+      clipboard.copyString(selectedText, this._view.document);
+      NativeWindow.toast.show(Strings.browser.GetStringFromName("selectionHelper.textCopied"), "short");
+    }
+    this._closeSelection();
+  },
 
-    this._activeType = this.TYPE_NONE;
-    sendMessageToJava({
-      type: "TextSelection:HideHandles",
-      handles: [this.HANDLE_TYPE_START, this.HANDLE_TYPE_END]
-    });
+  shareSelection: function sh_shareSelection() {
+    let selectedText = this._getSelectedText();
+    if (selectedText.length) {
+      sendMessageToJava({
+        type: "Share:Text",
+        text: selectedText
+      });
+    }
+    this._closeSelection();
+  },
 
+  /*
+   * Shuts SelectionHandler down.
+   */
+  _closeSelection: function sh_closeSelection() {
+    // Bail if there's no active selection
+    if (this._activeType == this.TYPE_NONE)
+      return;
 
-    let selectedText = "";
-    let pointInSelection = false;
-    if (this._view) {
+    if (this._activeType == this.TYPE_SELECTION) {
       let selection = this.getSelection();
       if (selection) {
-        // Get the text before we clear the selection!
-        selectedText = selection.toString().trim();
-
-        // Also figure out if the point is in the selection before we clear it.
-        if (arguments.length == 2 && this._pointInSelection(aX, aY))
-          pointInSelection = true;
-
+        // Remove the listener before calling removeAllRanges() to avoid
+        // recursively notifying the listener.
+        selection.QueryInterface(Ci.nsISelectionPrivate).removeSelectionListener(this);
         selection.removeAllRanges();
-        selection.QueryInterface(Ci.nsISelectionPrivate).removeSelectionListener(this);
       }
     }
 
-    // Only try copying text if there's text to copy!
-    if (pointInSelection && selectedText.length) {
-      let element = ElementTouchHelper.anyElementFromPoint(aX, aY);
-      // Only try copying text if the tap happens in the same view
-      if (element.ownerDocument.defaultView == this._view) {
-        let clipboard = Cc["@mozilla.org/widget/clipboardhelper;1"].getService(Ci.nsIClipboardHelper);
-        clipboard.copyString(selectedText, element.ownerDocument);
-        NativeWindow.toast.show(Strings.browser.GetStringFromName("selectionHelper.textCopied"), "short");
-      }
-    }
+    this._activeType = this.TYPE_NONE;
 
-    this._cleanUp();
+    sendMessageToJava({ type: "TextSelection:HideHandles" });
 
-    return selectedText;
-  },
-
-  _cleanUp: function sh_cleanUp() {
     this._removeObservers();
     this._view.removeEventListener("pagehide", this, false);
     this._view.removeEventListener("keydown", this, false);
     this._view.removeEventListener("blur", this, true);
-    this._activeType = this.TYPE_NONE;
     this._view = null;
     this._target = null;
     this._isRTL = false;
     this.cache = null;
   },
 
   _getViewOffset: function sh_getViewOffset() {
     let offset = { x: 0, y: 0 };
@@ -491,26 +484,16 @@ var SelectionHandler = {
     this.positionHandles();
 
     sendMessageToJava({
       type: "TextSelection:ShowHandles",
       handles: [this.HANDLE_TYPE_MIDDLE]
     });
   },
 
-  hideThumb: function sh_hideThumb() {
-    this._activeType = this.TYPE_NONE;
-    this._cleanUp();
-
-    sendMessageToJava({
-      type: "TextSelection:HideHandles",
-      handles: [this.HANDLE_TYPE_MIDDLE]
-    });
-  },
-
   positionHandles: function sh_positionHandles() {
     let scrollX = {}, scrollY = {};
     this._view.top.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindowUtils).getScrollXY(false, scrollX, scrollY);
 
     // the checkHidden function tests to see if the given point is hidden inside an
     // iframe/subdocument. this is so that if we select some text inside an iframe and
     // scroll the iframe so the selection is out of view, we hide the handles rather
     // than having them float on top of the main page content.
--- a/mobile/android/chrome/content/browser.js
+++ b/mobile/android/chrome/content/browser.js
@@ -5381,18 +5381,17 @@ var ClipboardHelper = {
 
   get clipboard() {
     delete this.clipboard;
     return this.clipboard = Cc["@mozilla.org/widget/clipboard;1"].getService(Ci.nsIClipboard);
   },
 
   copy: function(aElement, aX, aY) {
     if (SelectionHandler.shouldShowContextMenu(aX, aY)) {
-      // Passing coordinates to endSelection takes care of copying for us
-      SelectionHandler.endSelection(aX, aY);
+      SelectionHandler.copySelection();
       return;
     }
 
     let selectionStart = aElement.selectionStart;
     let selectionEnd = aElement.selectionEnd;
     if (selectionStart != selectionEnd) {
       string = aElement.value.slice(selectionStart, selectionEnd);
       this.clipboardHelper.copyString(string, aElement.ownerDocument);
@@ -5405,21 +5404,17 @@ var ClipboardHelper = {
     SelectionHandler.startSelection(aElement, aX, aY);
   },
 
   selectAll: function(aElement, aX, aY) {
     SelectionHandler.selectAll(aElement, aX, aY);
   },
 
   share: function() {
-    let selectedText = SelectionHandler.endSelection();
-    sendMessageToJava({
-      type: "Share:Text",
-      text: selectedText
-    });
+    SelectionHandler.shareSelection();
   },
 
   paste: function(aElement) {
     if (!aElement || !(aElement instanceof Ci.nsIDOMNSEditableElement))
       return;
     let target = aElement.QueryInterface(Ci.nsIDOMNSEditableElement);
     target.editor.paste(Ci.nsIClipboard.kGlobalClipboard);
     target.focus();