Closed Bug 1480031 Opened Last year Closed 3 months ago

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

Categories

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

Firefox 63
ARM
Android
defect

Tracking

()

VERIFIED FIXED
Firefox 70
Tracking Status
firefox-esr60 --- wontfix
firefox-esr68 --- verified
firefox61 --- wontfix
firefox62 --- wontfix
firefox63 - wontfix
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- wontfix
firefox67 --- wontfix
firefox68 --- wontfix
firefox69 --- verified
firefox70 --- verified

People

(Reporter: sflorean, Assigned: petru)

Details

(Whiteboard: [fennec68.1])

Attachments

(4 files)

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".
Sorina, 62 is marked as affected, can you confirm? Also is this a regression since 61? Thanks
Flags: needinfo?(sorina.florean)
(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.
Flags: needinfo?(sorina.florean)
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.
Priority: -- → P2
Assignee: nobody → petru.lingurar
Status: NEW → ASSIGNED
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
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.

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

Flags: needinfo?(sorina.florean)

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).
Flags: needinfo?(sorina.florean)

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.

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.

Keywords: checkin-needed

Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/835ac3ae20ba
Truncate floating menu labels if they overflow screen width; r=VladBaicu

Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 70

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

Flags: qe-verify+

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

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).

Flags: qe-verify+

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:
Flags: needinfo?(petru.lingurar)
Attachment #9077326 - Flags: approval-mozilla-esr68?
Attachment #9077326 - Flags: approval-mozilla-beta?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

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.

Attachment #9077326 - Flags: approval-mozilla-esr68?
Attachment #9077326 - Flags: approval-mozilla-esr68+
Attachment #9077326 - Flags: approval-mozilla-beta?
Attachment #9077326 - Flags: approval-mozilla-beta+

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!

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+

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

Keywords: regression

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

Keywords: regression
Whiteboard: [fennec68.1]
You need to log in before you can comment on or make changes to this bug.