Clipboard ActionBar missing on URL field in Android 4.3

VERIFIED FIXED in Firefox 24

Status

()

Firefox for Android
General
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: aaronmt, Assigned: sriram)

Tracking

({reproducible})

23 Branch
Firefox 26
ARM
Android
reproducible
Points:
---

Firefox Tracking Flags

(firefox22 wontfix, firefox23 wontfix, firefox24 verified, firefox25 verified, firefox26 verified, relnote-firefox 23+, fennec+)

Details

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
See video: http://www.youtube.com/watch?v=JUja4EBJzSA&hd=1

The ActionBar appears and vanishes very quickly.

--
Nightly (07/02)
Samsung Galaxy S4 (Android 4.3)
(Reporter)

Updated

4 years ago
Keywords: reproducible
Assignee: nobody → sriram
tracking-fennec: ? → +
(Assignee)

Comment 1

4 years ago
There is no official 4.3 build yet :P
(Reporter)

Comment 2

4 years ago
Renominating because it affects all devices on Androids 4.3. I see this on my Nexus 4 (Android 4.3) too.
tracking-fennec: + → ?
(Reporter)

Comment 3

4 years ago
Confirming seeing this on my Nexus 7 with Android 4.3 and Galaxy S4 (Google Edition) 4.3.
tracking-fennec: ? → +
(Reporter)

Comment 4

4 years ago
Sriram, did you update your devices to 4.3?
(Assignee)

Comment 5

4 years ago
I haven't got 4.3 on any device yet. I tried downloading and force updating Nexus 4. That didn't work either.
relnote-firefox: --- → ?
(Reporter)

Comment 6

4 years ago
For clarification for a release note; users on all versions of Firefox running Android 4.3 will not be able to copy, cut, or paste into the address-bar.
I am able to paste as that action shows on long press. However cut/copy/select all are unavailable.
(Assignee)

Comment 8

4 years ago
Created attachment 784629 [details] [diff] [review]
Patch

We've been having code that we actually didn't want! :sigh:
This works on 4.2.2 and 4.3. Please check in other 3.0+ devices.
Attachment #784629 - Flags: review?(mark.finkle)
Added to Known Issues as:

Unresolved
Users on Android 4.3 will not be able to copy, cut, or paste into the address bar (see 889562)


Please contact release-mgmt@mozilla.com with any questions or corrections.
relnote-firefox: ? → 23+
Comment on attachment 784629 [details] [diff] [review]
Patch

># HG changeset patch
># User Sriram Ramasubramanian <sriram@mozilla.com>
># Date 1375396761 25200
># Node ID 325adb83315e095450730f38a770a0267fc42a4d
># Parent  05d3797276d3d8c7d10ddc66e95c99352c2e3cc9
>Bug 889562: Action-mode not showing in Android 4.3 [r=mfinkle]
>
>diff --git a/mobile/android/base/AwesomeBar.java b/mobile/android/base/AwesomeBar.java
>         setContentView(R.layout.awesomebar);
> 
>-        // ActionBar's isShowing() method returns true in its initial state,
>-        // so that it cannot be drawn until hide is called.
>-        if (Build.VERSION.SDK_INT >= 11) {
>-            getActionBar().hide();
>-        }

This code was added in bug 889555 recently and was marked as "verified". Why would removing it not break bug 889555 again?

>-        mText.setOnLongClickListener(new View.OnLongClickListener() {
>-            @Override
>-            public boolean onLongClick(View v) {
>-                if (Build.VERSION.SDK_INT >= 11) {
>-                    CustomEditText text = (CustomEditText) v;
>-
>-                    if (text.getSelectionStart() == text.getSelectionEnd())
>-                        return false;
>-
>-                    getActionBar().show();
>-                    return false;
>-                }
>-
>-                return false;
>-            }
>-        });
>-
>-        mText.setOnSelectionChangedListener(new CustomEditText.OnSelectionChangedListener() {
>-            @Override
>-            public void onSelectionChanged(int selStart, int selEnd) {
>-                if (Build.VERSION.SDK_INT >= 11 && selStart == selEnd) {
>-                    getActionBar().hide();
>-                }
>-            }
>-        });
>-

>-
>-        if (Build.VERSION.SDK_INT >= 11) {
>-            getActionBar().hide();
>-        }

These blocks of code were added in bug 770928. Why can we safely remove these now?
(Assignee)

Comment 11

4 years ago
Comment on attachment 784629 [details] [diff] [review]
Patch

Review of attachment 784629 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/AwesomeBar.java
@@ -92,5 @@
> -        // so that it cannot be drawn until hide is called.
> -        if (Build.VERSION.SDK_INT >= 11) {
> -            getActionBar().hide();
> -        }
> -

This was added because, android:visibility of ActionBar and its show()/hide() method doesn't match. We make the visibility "gone" in XML. However, android thinks that the ActionBar is showing when it starts this activity. Hence the Contextual Action Bar (CAB) was not shown. Now that I'm removing action-bar in AwesomeBar, this piece is no longer needed.

@@ -232,5 @@
>                  }
>              }
>          });
>  
> -        mText.setOnLongClickListener(new View.OnLongClickListener() {

We needn't do this. Android shows the CAB by default on any text-view, as it was just a replacement for the long press menu. So this listener is not needed actually.

@@ -238,5 @@
> -            public boolean onLongClick(View v) {
> -                if (Build.VERSION.SDK_INT >= 11) {
> -                    CustomEditText text = (CustomEditText) v;
> -
> -                    if (text.getSelectionStart() == text.getSelectionEnd())

This piece is strange. In 4.3, when you tap on the text view, and then long press, more often, the start and end values are same! :O Android bug!

@@ -241,5 @@
> -
> -                    if (text.getSelectionStart() == text.getSelectionEnd())
> -                        return false;
> -
> -                    getActionBar().show();

No ActionBar anymore.

@@ -253,5 @@
> -        mText.setOnSelectionChangedListener(new CustomEditText.OnSelectionChangedListener() {
> -            @Override
> -            public void onSelectionChanged(int selStart, int selEnd) {
> -                if (Build.VERSION.SDK_INT >= 11 && selStart == selEnd) {
> -                    getActionBar().hide();

Since we don't worry about selection. This isn't needed. Selection handles and selection are taken care of my Android.

@@ -745,5 @@
>              updateGoButton(text);
>          }
> -
> -        if (Build.VERSION.SDK_INT >= 11) {
> -            getActionBar().hide();

There is no ActionBar.

::: mobile/android/base/resources/values-v11/styles.xml
@@ -52,5 @@
> -    <!-- AwesomeBar ActionBar -->
> -    <style name="ActionBar.AwesomeBar">
> -         <item name="android:displayOptions">showCustom</item>
> -         <item name="android:customNavigationLayout">@layout/awesomebar_actionbar</item>
> -         <item name="android:visibility">gone</item>

This doesn't directly reflect the show()/hide() of ActionBar. We load a custom layout and then make it "gone". This wasn't needed actually.

::: mobile/android/base/resources/values-v11/themes.xml
@@ -25,5 @@
>      <style name="GeckoAwesomeBarBase" parent="GeckoBase">
>          <item name="android:windowBackground">@color/background_normal</item>
> -        <item name="android:windowActionBar">true</item>
> -        <item name="android:windowNoTitle">false</item>
> -        <item name="android:actionBarStyle">@style/ActionBar.AwesomeBar</item>

The idea for this patch came from new about:home. In new about:home, the long press (after a series of hit and miss, as we show a different menu on the url-bar) shows CAB. But the GeckoApp doesn't have an action-bar. It's windowActionBar property is false. If that would work fine, then we actually wouldn't be needing it in old AwesomeBar. And I tried it .. it works :D.

I still don't recollect why I added the ActionBar for AwesomeBar in the first place. May be the honeycomb or ICS might depend on it. I would probably try it tomorrow and update the results. In that case, we can be partial in having the logic for only those devices that have the ActionBar.
(Reporter)

Comment 12

4 years ago
Can we get this for first beta?
Status: NEW → ASSIGNED
Go to build for the first beta went off at 11 pacific.
Comment on attachment 784629 [details] [diff] [review]
Patch

This will need a lot of testing around how the AwesomeBar opens and reacts to focus/input.
Attachment #784629 - Flags: review?(mark.finkle) → review+
(Assignee)

Comment 15

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/968357fb185f
(Reporter)

Updated

4 years ago
status-firefox26: --- → affected
https://hg.mozilla.org/mozilla-central/rev/968357fb185f
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26

Updated

4 years ago
status-firefox26: affected → fixed
(Reporter)

Comment 17

4 years ago
Tested this on two phones, mini-tablet, tablet

* LG Nexus 4 (Android 4.3), Asus Nexus 7 (Android 4.3), HTC One (Android 4.2), Samsung Galaxy Tab 3 10" (Android 4.2)
↳ Confirmed restoration of basic action-bar functionality

Adrian is testing for no regressions on Android 4.0/3.0; otherwise this looks good to me. I'm comfortable with this heading to release to restore basic functionality that is lost to those that upgraded.
status-firefox26: fixed → verified
I tested on Asus EEE Transformer TF101 (Android 4.0.4), Samsung Galaxy Note (Android 4.0.3) and Acer Iconia Tab (Android 3.2). The cut/copy/paste action bar works without any issues on Nightly 26.0a1 2013-08-08.
(Reporter)

Updated

4 years ago
Status: RESOLVED → VERIFIED
(Assignee)

Comment 19

4 years ago
Comment on attachment 784629 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Long long ago we didn't know Android would do this for free.
User impact if declined: Can't cut/copy/paste. 
Testing completed (on m-c, etc.): Verified on 08/08.
Risk to taking this patch (and alternatives if risky): None. It's android's default implementation.
String or IDL/UUID changes made by this patch: None.
Attachment #784629 - Flags: approval-mozilla-beta?
Attachment #784629 - Flags: approval-mozilla-aurora?
Comment on attachment 784629 [details] [diff] [review]
Patch

This is worth the risk, but deserves QA verification.
Attachment #784629 - Flags: approval-mozilla-beta?
Attachment #784629 - Flags: approval-mozilla-beta+
Attachment #784629 - Flags: approval-mozilla-aurora?
Attachment #784629 - Flags: approval-mozilla-aurora+

Updated

4 years ago
Keywords: qawanted, verifyme
https://hg.mozilla.org/releases/mozilla-aurora/rev/258af60eb930
https://hg.mozilla.org/releases/mozilla-beta/rev/9a5b3a4ebcbe
status-firefox22: affected → wontfix
status-firefox23: affected → wontfix
status-firefox24: affected → fixed
status-firefox25: affected → fixed
(Reporter)

Updated

4 years ago
status-firefox24: fixed → verified
status-firefox25: fixed → verified
Keywords: qawanted, verifyme
You need to log in before you can comment on or make changes to this bug.