1050780 - Browser menu missing dividers in Android L Preview

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Aaron Train [:aaronmt]]

Attachment: screenshot.png

On Android L Preview (LPV81C), the in-browser menu seems to be missing a bunch of dividers for each menu item.

See screenshot.

--
Nexus 7 (L Preview LPV81C)

Comment 1

[Aaron Train [:aaronmt]]

Similar to bug 963249, the dividers are missing only when the menu is opened on about:home again

Comment 2

[Aaron Train [:aaronmt]]

Attachment: screenshot-content.png

Actually I retract that. Here's an example in content with the 'Page' menu missing dividers.

Comment 3

[Lucas Rocha (:lucasr)]

For the record, it seems we've had similar issues before. See bug 963249.

Comment 4

[:Margaret Leibovic]

Let's track this to get it in ASAP, and we can look to uplift if there's a low-risk fix.

Comment 5

[Lucas Rocha (:lucasr)]

Attachment: Avoid disabled items in GeckoMenu's adapter (r=margaret)

Comment 6

[Lucas Rocha (:lucasr)]

Comment on attachment 8481442 [details] [diff] [review]
Avoid disabled items in GeckoMenu's adapter (r=margaret)

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

I initially thought this was an Android bug but it seems this is the intended behaviour in ListView. Dividers are not drawn around items disabled in the adapter. See:
https://github.com/android/platform_frameworks_base/blob/master/core/java/android/widget/ListView.java#L3274

I had to change the background selector to only show pressed state on enabled menu items as the ListView's OnItemClickListener will still set pressed state on the disabled menu item.

Comment 7

[Lucas Rocha (:lucasr)]

https://hg.mozilla.org/integration/fx-team/rev/8fc296b9a1df

Comment 8

[Ekanan Ketunuti]

https://hg.mozilla.org/mozilla-central/rev/8fc296b9a1df

Comment 9

[Lucas Rocha (:lucasr)]

Comment on attachment 8481442 [details] [diff] [review]
Avoid disabled items in GeckoMenu's adapter (r=margaret)

Approval Request Comment
[Feature/regressing bug #]: the Android L release
[User impact if declined]: menu looks visually broken when certain items are disabled.
[Describe test coverage new/current, TBPL]: Local testing on Kitkat/L devices only. Let's bake this in Nightly for a few days and then uplift.
[Risks and why]: Low, simply avoid disabling items in the menu's adapter so that dividers are always drawn before/after them.
[String/UUID change made/needed]: n/a

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/3133016b4867
https://hg.mozilla.org/releases/mozilla-beta/rev/7cf512b6b64c