968182 - Share options can't be scrolled all the way - HTC Sensation 4G

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Ioana Chiorean]

Attachment: Screenshot

Device: HTC Sensation 4G
OS: Android 4.0.3
Firefox Aurora  29.0a2 02/05 and Nightly 30.0a1

Steps:
1. Open app and go to addons.mozilla.org (or any page that can be shared)
2. press Menu -> quick share button (the 3 dots connected )

Expected results:
- The share options can be scrolled all the way

Actual results:
- In portrait mode the list can be panned all the way  (last 3 optioned not displayed)

Note:
- This was not reproduce on other phones both with hardware menu and 4.0+ Os nor 2.x OS
- Please see video: https://www.youtube.com/watch?v=Z7Znp2kwS1I
- the empty space is already logged in Bug 965817

Comment 1

[Brad Lassey [:blassey] (use needinfo?)]

tracking+ for now, Margaret is ordering a phone. once we have on in house, let's look to see if this is a recent regression (since 25) and renom if it is.

Comment 2

[:Margaret Leibovic]

Filed Service Now request REQ0022273.

Comment 3

[Wesley Johnston (:wesj)]

Attachment: Patch

We do some strange measurement things that seem to tickle a bug in the HTC's menu implementation and result in our ListView being larger than its parent. This makes us not do that for these types of phones (BUT causes our Share menu to scroll to the height of the screen on non-buggy devices...)

I tried to force the parent to reflow after (again) we were measured via ViewParent.requestLayout(), but haven't had much success...

Comment 4

[Lucas Rocha (:lucasr)]

Comment on attachment 8387455 [details] [diff] [review]
Patch

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

Forgot to qref? :-)

Comment 5

[:Margaret Leibovic]

Thanks for taking a look at this!

Comment 6

[Wesley Johnston (:wesj)]

Attachment: Patch

Whoops.This should have code inside!

Comment 7

[Lucas Rocha (:lucasr)]

Comment on attachment 8387665 [details] [diff] [review]
Patch

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

Just checking if a simpler change is enough to fix this bug.

::: mobile/android/base/menu/MenuPanel.java
@@ -29,5 @@
>      }
>  
>      @Override
> -    protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
> -        super.onMeasure(widthMeasureSpec, heightMeasureSpec);

This first super.onMeasure() call is unnecessary and look suspicious. Have you tried simply removing it to see if it fixes the problem?

Comment 8

[Wesley Johnston (:wesj)]

Attachment: Alt Patch

Removing the measure call doesn't seem to help. Here's an alternative that forces us to close the menu so that when we reopen it the window gets remeasured. I'm not sure why this was marked for non-hardware menu phones only before, and the blame is gone.

Maybe mfinkle remembers?

Comment 9

[Lucas Rocha (:lucasr)]

Comment on attachment 8388765 [details] [diff] [review]
Alt Patch

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

I'd prefer this approach (if that doesn't cause any side effects). It's probably a good idea to update the comment explaining why this is necessary.

Comment 10

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Wesley Johnston (:wesj) from comment #8)
> Created attachment 8388765 [details] [diff] [review]
> Alt Patch
> 
> Removing the measure call doesn't seem to help. Here's an alternative that
> forces us to close the menu so that when we reopen it the window gets
> remeasured. I'm not sure why this was marked for non-hardware menu phones
> only before, and the blame is gone.
> 
> Maybe mfinkle remembers?

The code seems to have been added for bug 794581

http://hg.mozilla.org/mozilla-central/rev/33f6113f5a8b

Comment 11

[Wesley Johnston (:wesj)]

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

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e7f97fa8a86b

Comment 13

[Mark Finkle (:mfinkle) (use needinfo?)]

Let's try to make sure bug 794581 is not regressed.

Comment 14

[Aaron Train [:aaronmt]]

(In reply to Mark Finkle (:mfinkle) from comment #13)
> Let's try to make sure bug 794581 is not regressed.

It did