Last Comment Bug 770928 - Unable to invoke the text-interaction bar (ActionMode) on a URL in Android 4.1
: Unable to invoke the text-interaction bar (ActionMode) on a URL in Android 4.1
: regression, relnote
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: Trunk
: ARM Android
-- normal with 1 vote (vote)
: Firefox 17
Assigned To: Sriram Ramasubramanian [:sriram]
: Sebastian Kaspari (:sebastian)
: 760160 (view as bug list)
Depends on: 778438
  Show dependency treegraph
Reported: 2012-07-04 10:00 PDT by Aaron Train [:aaronmt]
Modified: 2012-10-03 14:34 PDT (History)
10 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch (6.64 KB, patch)
2012-07-26 15:43 PDT, Sriram Ramasubramanian [:sriram]
no flags Details | Diff | Splinter Review
Patch (6.78 KB, patch)
2012-07-26 15:56 PDT, Sriram Ramasubramanian [:sriram]
mbrubeck: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description User image Aaron Train [:aaronmt] 2012-07-04 10:00:53 PDT
Long-tap and hold on the current URL. 

ER: ICS/Jellybean text-interaction bar - (context-menu, 2.2/2.3)
AR: Nothing

Bring back the text-interaction bar.

Samsung Galaxy Nexus (Android 4.1)
Nightly (07/04)
Comment 1 User image Aaron Train [:aaronmt] 2012-07-04 10:07:46 PDT
My mistake, looks like we have a 4.1 issue here.
Comment 2 User image Aaron Train [:aaronmt] 2012-07-04 11:06:58 PDT
Sriram, do we need startActionMode?
Comment 3 User image Aaron Train [:aaronmt] 2012-07-16 11:30:06 PDT
This will be a pain-point for a functional loss and negative review for JB users.
Comment 4 User image Sriram Ramasubramanian [:sriram] 2012-07-17 14:42:09 PDT
Actual STR:
1. Tap on the URL bar
2. Long tap on the "editable" url.
Comment 5 User image Sriram Ramasubramanian [:sriram] 2012-07-17 14:44:15 PDT
We currently don't use ActionBar in AwesomeBar. It's hidden in AwesomeBar.
  When we long press, android brings up the text-interaction bar -- even when we make the ActionBar visibility to be "gone".

In JB:
  When we long press, android doesn't bring up the text-interaction bar -- as the visibility of the ActionBar is "gone".

Now, we can override the long press on the editable text to show the action-bar.
But, we won't know when the user is done with the options android gives, as we don't own the ActionMode.Callback to monitor the change of ActionBar. So, we cannot close it.
Comment 6 User image Brad Lassey [:blassey] (use needinfo?) 2012-07-18 13:41:42 PDT
tracking for 15 just because we'll want to make an explicit call about uplifting when we have a patch to evaluate
Comment 7 User image Aaron Train [:aaronmt] 2012-07-20 13:26:03 PDT
Nexus S users just got 4.1.1 [1], just got it on my phone. Confirming same issue on my Nexus S.

Comment 8 User image Sriram Ramasubramanian [:sriram] 2012-07-25 11:54:48 PDT
*** Bug 760160 has been marked as a duplicate of this bug. ***
Comment 9 User image Lukas Blakk [:lsblakk] use ?needinfo 2012-07-25 12:02:08 PDT
Carrying forward tracking from bug 760160 so we can keep this on our radar.
Comment 10 User image Sriram Ramasubramanian [:sriram] 2012-07-26 15:43:33 PDT
Created attachment 646375 [details] [diff] [review]

This patch finally fixes it.
Basic problems:
 JB is strict with ActionBar! If we specify the visibility as "gone", it won't show text interaction.
 We don't own the ActionMode when it is shown, and hence we cannot hide it when text-editing is done.

 On long press, show the action-bar. Android takes care of the action-mode there.
 When we know the text has changed -- possibly by cut/paste,
 or when the selection has changed -- by dismissing the selected text -- possibly by copy/done/tapping on EditText
 hide the actionbar.
Comment 11 User image Sriram Ramasubramanian [:sriram] 2012-07-26 15:56:57 PDT
Created attachment 646386 [details] [diff] [review]

This fixes one other bug.
Comment 12 User image Matt Brubeck (:mbrubeck) 2012-07-26 16:01:33 PDT
Comment on attachment 646375 [details] [diff] [review]

Review of attachment 646375 [details] [diff] [review]:

::: mobile/android/base/
@@ +217,5 @@
> +            @Override
> +            public boolean onLongClick(View v) {
> +                if (Build.VERSION.SDK_INT >= 11 && ((CustomEditText) v).getText().length() > 0) {
> +                    getActionBar().show();
> +                    return false;

Should we "return true" here?

::: mobile/android/base/
@@ +26,5 @@
>      public void setOnKeyPreImeListener(OnKeyPreImeListener listener) {
>          mOnKeyPreImeListener = listener;
>      }
> +    OnSelectionChangedListener mOnSelectionChangedListener;

Minor nit: Please move all the new code below the onKeyPreIme() method (so all the onKeyPreImeListener code remains together).
Comment 13 User image Aaron Train [:aaronmt] 2012-07-26 16:08:23 PDT
QA Note:

Look for no functional loss on all other supported Android versions.
Comment 14 User image Sriram Ramasubramanian [:sriram] 2012-07-26 23:59:08 PDT

There are 11+ checks everywhere. So it won't affect.
Comment 15 User image Ed Morley [:emorley] 2012-07-27 08:14:24 PDT
Comment 16 User image Aaron Train [:aaronmt] 2012-07-30 07:22:21 PDT
This broke Honeycomb in bug 778438.
Comment 17 User image Lukas Blakk [:lsblakk] use ?needinfo 2012-07-30 16:43:19 PDT
please nominate for uplift if this is low-risk enough to consider. i've also tracked bug 778438 so we can ensure we take the honeycomb breakage fix at the same time.
Comment 18 User image Aaron Train [:aaronmt] 2012-07-31 07:39:54 PDT
Uplift! Go-go-go
Comment 19 User image Sriram Ramasubramanian [:sriram] 2012-07-31 17:12:17 PDT
Comment on attachment 646386 [details] [diff] [review]

[Approval Request Comment]
Bug caused by (feature/regressing bug #): -
User impact if declined: Users cannot cut-copy-paste on awesomescreen.
Testing completed (on m-c, etc.): Landed in m-c on 07/27
Risk to taking this patch (and alternatives if risky): Bug 760160.
String or UUID changes made by this patch: None.
Comment 20 User image Lukas Blakk [:lsblakk] use ?needinfo 2012-08-01 17:41:23 PDT
Comment on attachment 646386 [details] [diff] [review]

Not sure why the possible fallout from this bug is linking to a bug that is resolved as duplicate of _this_ bug, but I assume that means any fallout from this patch will be tracking in this bug.
Comment 21 User image Sriram Ramasubramanian [:sriram] 2012-08-07 11:35:12 PDT
Pushed to aurora:
Comment 22 User image Sriram Ramasubramanian [:sriram] 2012-08-07 11:51:26 PDT
Pushed to beta:
Comment 23 User image Cristian Nicolae (:xti) 2012-09-27 02:35:40 PDT
Indeed, it's fixed on all branches.

Device: Galaxy Note
OS: Android 4.1.1
Comment 24 User image Martijn Wargers [:mwargers] 2012-10-03 14:34:51 PDT
I need 3 long taps before I finally get the text-interaction bar to appear for the url bar. That seems very excessive, so I filed bug 797565 for it.

Note You need to log in before you can comment on or make changes to this bug.