768665 - Add context menu to selected text

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Text Selection, defect)

Description

[:Margaret Leibovic]

It should include "Copy", "Select All", and "Share".

Comment 1

[:Margaret Leibovic]

Attachment: patch

Comment 2

[Aaron Train [:aaronmt]]

What does Select All do on already selected text?

Comment 3

[:Margaret Leibovic]

(In reply to Aaron Train [:aaronmt] from comment #2)
> What does Select All do on already selected text?

It selects the entire page (like cmd+a on desktop). The action bar on selection on Chrome also has a button that does this, so it seems useful.

Comment 4

[Yann Brelière]

Would this still be needed if the action bar (Bug 768667) was implemented?

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Yann Brelière from comment #4)
> Would this still be needed if the action bar (Bug 768667) was implemented?

For Froyo and Gingerbread it would be needed. We would not support the ActionBar there.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 636937 [details] [diff] [review]
patch

>diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js

>+  showContextMenu: function sh_showContextMenu(aX, aY) {

>+      case SELECT_ALL:
>+        let selectionController = this._view.QueryInterface(Ci.nsIInterfaceRequestor).

>+      case SHARE:
>+        let selectedText = this.endSelection();

Can you add {} since you are making local vars in the case blocks?

        case SELECT_ALL: {
          let selectionController = this._view.QueryInterface(Ci.nsIInterfaceRequestor).
          ...
          break;
        }


>+    return selectedText;

Is this used? or is this just a handy thing to do?

Comment 7

[:Margaret Leibovic]

(In reply to Mark Finkle (:mfinkle) from comment #6)

>         case SELECT_ALL: {
>           let selectionController =
> this._view.QueryInterface(Ci.nsIInterfaceRequestor).
>           ...
>           break;
>         }
> 
> 
> >+    return selectedText;
> 
> Is this used? or is this just a handy thing to do?

I added it so that it could be used for the share method. It's a little tricksy for my tastes, but I didn't want to try to refactor endSelection to factor out the code that works with the selection.

Comment 8

[:Margaret Leibovic]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c64440f55c79

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c64440f55c79

Comment 10

[:Margaret Leibovic]

Uplifted to aurora as part of a roll-up patch:
https://hg.mozilla.org/releases/mozilla-aurora/rev/2fb0a358eaf6