1480031 - Three dots menu from selection bar not displayed when the search engine name is too long

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P2)

Description

[Sorina Florean [:sflorean]]

Attachment: Screenshot_20180801-122408.png

Environment: 
Device: Nokia 6(Android 7.1.1);
Build: Nightly 63.0a1 (2018-07-31);

Steps to reproduce:
1. Add IMDB as a search engine (leave all the name IMDB - Movies, TV and Celebrities - IMDB);
2. Open an article and long tap to bring up the selection menu;
3. Tap on three dots menu.

Expected result:
Three dots menu with the option "IMDB search" is displayed.

Actual result:
Three dots menu is not displayed.

Notes:
Not reproducible when the name of the search engine is "IMDB".

Comment 1

[Pascal Chevrel:pascalc]

Sorina, 62 is marked as affected, can you confirm? Also is this a regression since 61? Thanks

Comment 2

[Sorina Florean [:sflorean]]

(In reply to Pascal Chevrel:pascalc from comment #1)
> Sorina, 62 is marked as affected, can you confirm? Also is this a regression
> since 61? Thanks

Tested on 59.0a1 from 2018-01-01 and the issue is reproducible. I will set the flag for 61 affected as well. 

One extra step that I forgot in order to reproduce the issue: put IMDB as default engine search.

Comment 3

[Pascal Chevrel:pascalc]

Thanks. This is thus not a recent regression, I will happily take a fix for 63 but no need to track this bug for our upcoming releases.

Comment 4

[Petru-Mugurel Lingurar [:petru]]

Attachment: too_long_search_title.png

Comment 5

[Petru-Mugurel Lingurar [:petru]]

Indeed, on lower density screens it is possible that a long search engine name would overflow the Rectangle in which it is to be written.

I initially went with calculating here [1] where to break the search engine name and add an ellipsis that was to be followed by " Search" as in browser.properties [2] but then remembered that the localizers are free to modify that.
(For example in place of "%S search" they can say "Search with %S" if it makes more sense in a particular language)
So just cutting the first part of the search engine name and adding "... Search" would break things.

As such, because at runtime we cannot know where to cut the text I think the only solution would remain to only allow a new search engine if it has a decent length title (users can edit this - attachment 9029637 [details]). Let's say an arbitrary value of 20 characters.


[1] https://searchfox.org/mozilla-central/rev/3fdc51e66c9487b39220ad58dcee275fca070ccd/mobile/android/base/java/org/mozilla/gecko/text/FloatingActionModeCallback.java#45
[2] https://searchfox.org/mozilla-central/rev/3fdc51e66c9487b39220ad58dcee275fca070ccd/mobile/android/locales/en-US/chrome/browser.properties#305

Comment 6

[Liz Henry (:lizzard) (relman/hg->git project)]

Too late to fix in 64. Marking this issue as fix-optional for 65; if you land a patch in nightly and think it's low-risk for beta, please request uplift.

Comment 7

[Petru-Mugurel Lingurar [:petru]]

Sorina, is this still reproducing?
I'm not seeing it anymore - https://drive.google.com/file/d/1egM50fBfY4u5PI4xKXFN4B4LP2fd7MTj

Comment 8

[Eliza Balazs]

Attachment: Screenshot_20190408-164926.png

Hi!

I tested this following the steps from the Description on the latest versions of Nightly 68.0a1 (2019-04-07), Beta 67.0b7, Release 66.0.2 with HTC 10 (Android 8.0.0), Google Pixel (Android Q) and I can reproduce the issue.

Note:

• This is not reproducible on Nexus 9 (Android 7.1.1; Tablet).

Comment 9

[Petru-Mugurel Lingurar [:petru]]

This seems like an issue in Android for which I've filed an issue - https://issuetracker.google.com/issues/137169336

Will continue to work on an workaround.

Comment 10

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1480031 - Truncate floating menu labels if they overflow screen width; r?VladBaicu

There's currently a bug in Android's framework that manifests by placing the
floating menu off-screen if a menu label overflows the menu's width.
https://issuetracker.google.com/issues/137169336
To overcome this we'll manually check and truncate any menu label that could
cause issues based on the floating menu style declared upstream.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/835ac3ae20ba
Truncate floating menu labels if they overflow screen width; r=VladBaicu

Comment 12

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/835ac3ae20ba

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

Comment 14

[Chris Peterson [:cpeterson]]

Petru, we'll want to uplift your menu fix to Fennec ESR 68.1 and Beta 69.

Comment 15

[Stefan Deiac]

Hello, I can confirm that the issue is fixed using Google Pixel (Android Q) with the latest Nightly from Treeherder(70.0a1 2019-07-18).

Comment 16

[Petru-Mugurel Lingurar [:petru]]

Comment on attachment 9077326 [details]
Bug 1480031 - Truncate floating menu labels if they overflow screen width; r?VladBaicu

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Long menu item title would make the menu overflow the screen and some options impossible to see / select.
• User impact if declined: Long menu item title would make the menu overflow the screen and some options impossible to see / select.
• Fix Landed on Version: 70
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Small change for only a few possible cases with popup menu items titles longer than 30 characters.
• String or UUID changes made by this patch:

Beta/Release Uplift Approval Request

• User impact if declined: Long menu item title would make the menu overflow the screen and some options impossible to see / select.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: comment #0
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Small change for only a few possible cases with popup menu items titles longer than 30 characters.
• String changes made/needed:

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9077326 [details]
Bug 1480031 - Truncate floating menu labels if they overflow screen width; r?VladBaicu

Fixes a bug causing the 3-dot menu to be truncated when long-pressing in some cases. Approved for Fennec 68.1b3.

Comment 18

[Razvan Maries]

https://hg.mozilla.org/releases/mozilla-beta/rev/43030692b97b

Comment 19

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-esr68/rev/c9ef4f8577c5

Comment 20

[Eliza Balazs]

Hi!
Verified as fixed on ESR 68.1b3, Beta 69.0b7 with OnePlus 5T (Android 9), HTC 10 (Android 8.0.0).
I will mark this issue as verified on Firefox esr68 and Firefox 69. Thanks!

Comment 21

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Comment 22

[Petru-Mugurel Lingurar [:petru]]

It's a bug in Android's framework, not a regression. See comment 9