Closed Bug 1352993 Opened 3 years ago Closed 3 years ago

Custom Tab: Action bar is not displayed when selecting a word

Categories

(Firefox for Android :: General, defect, P1)

55 Branch
ARM
Android
defect

Tracking

()

VERIFIED FIXED
Firefox 55
Tracking Status
firefox55 --- verified

People

(Reporter: sflorean, Assigned: walkingice)

References

(Blocks 2 open bugs)

Details

Attachments

(7 files)

Environment: 
Device: Huawei Honor (Android 5.1.1);
Build: Nightly 55.0a1 (2017-04-02);

Steps to reproduce:
1. Open a web page by custom tabs;
2. Long tap to select a word;

Expected result:
The selected word is highlighted. The action bar is displayed with the following options: share, search, copy and select all items.


Actual result:
The selected word is highlighted. The action bar is not displayed.

Notes:
- Long tap on a page to paste a string doesn't work either (e.g. google.com opened by custom tab and long tap on search field to paste something).
- On Chrome action bar is displayed.
Attached image screenshot_20170406.png
I use today's nightly, and I can see those options. It likely is fixed by another bug. Could you verify again once you have time slot, thanks!
Hi Sorina, Per comment1, Can you please help retest the bug ?
Thank you very much !
Flags: needinfo?(sorina.florean)
Tested again with Huawei MediaPad M2 (Android 5.1.1), and the issue is still reproducible on latest Nightly.

(In reply to Julian Chu [:walkingice] from comment #1)
> Created attachment 8855302 [details]
> screenshot_20170406.png
I think that is a Android 6 or more, right? The action bar is different for devices with Android 5 and 6.
Flags: needinfo?(sorina.florean)
P1 per the triage on April12
Priority: -- → P1
I used Nevin's phone (which has Android 5) and be able to reproduce this bug.
I just realized that what "ActionBar" did Sorina mean. The action-bar I thought[1] was the view consists url, title on the top of CustomTabsActivity. Now I know it should be the component for text selection.[2]

It seems we have to add the ViewFlipper to CustomTabsActivity.

[1] https://developer.android.com/training/appbar/index.html
[2] https://dxr.mozilla.org/mozilla-central/rev/f40e24f40b4c4556944c762d4764eace261297f5/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java#1328
Assignee: nobody → walkingice0204
In CustomTabsActivity, we use official ActionBar which supports action mode, and I would like to use it for text-selection.

Current ActionBarTextSelection has some custom classes, and those classes has very similar interface with Android classes. ie, ActionModeCompat vs. ActionMode. My first step is to get rid of those custom classes type. Less custom types means we have more flexibility to implement UI in Activity.

Then, use Presenter pattern, to provide UI response for text-selection.

The final step is to let CustomTabsActivity implements the presenter.
Comment on attachment 8859129 [details]
Bug 1352993 - Part 0, remove useless variable

https://reviewboard.mozilla.org/r/131176/#review133706
Attachment #8859129 - Flags: review?(s.kaspari) → review+
Comment on attachment 8859130 [details]
Bug 1352993 - Part 1, Generalize ActionModeCompat

https://reviewboard.mozilla.org/r/131178/#review133708

Nice!
Attachment #8859130 - Flags: review?(s.kaspari) → review+
Comment on attachment 8859131 [details]
Bug 1352993 - Part 2, Add presenter class which response text-selection

https://reviewboard.mozilla.org/r/131180/#review133710
Attachment #8859131 - Flags: review?(s.kaspari) → review+
Comment on attachment 8859132 [details]
Bug 1352993 - Part 3, CustomTabsActivity prvoides presenter

https://reviewboard.mozilla.org/r/131182/#review133712
Attachment #8859132 - Flags: review?(s.kaspari) → review+
(In reply to Sebastian Kaspari (:sebastian) from comment #16)
> Awesome!
> 
> Note that link reported one error:
> https://public-artifacts.taskcluster.net/QfhB02gtSVOwRCQdtP1h_A/0/public/
> android/lint/lint-results-automationDebug.html

Thanks!

We have to use ALWAYS to instead of IfRoom(otherwise, some buttons will be hided). So I added SuppressLint annotation to make linter happy.
Keywords: checkin-needed
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 483055072a06 -d 98d3518db0a4: rebasing 390454:483055072a06 "Bug 1352993 - Part 0, remove useless variable r=sebastian"
rebasing 390455:90706f52f943 "Bug 1352993 - Part 1, Generalize ActionModeCompat r=sebastian"
merging mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
rebasing 390456:00120d614925 "Bug 1352993 - Part 2, Add presenter class which response text-selection r=sebastian"
merging mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
merging mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
warning: conflicts while merging mobile/android/base/java/org/mozilla/gecko/GeckoApp.java! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
OK! rebased onto m-c
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/23e46ff286f5
Part 0, remove useless variable r=sebastian
https://hg.mozilla.org/integration/autoland/rev/3a8bbe748b38
Part 1, Generalize ActionModeCompat r=sebastian
https://hg.mozilla.org/integration/autoland/rev/bad2a571577a
Part 2, Add presenter class which response text-selection r=sebastian
https://hg.mozilla.org/integration/autoland/rev/229519a81ed2
Part 3, CustomTabsActivity prvoides presenter r=sebastian
Keywords: checkin-needed
Verified as fixed on latest Nightly build 55.0a1 from 2017-05-08.
Tested with Huawei Honor (Android 5.1.1) and when a word is selected the ActionBar is displayed. There is still an difference regarding the colors but for that issue I'll file a new bug since this was only for ActionBar.
See Also: → 1363043
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.