Closed Bug 968182 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 30
Tracking Status
fennec + ---

People

(Reporter: ioana.chiorean, Assigned: wesj)

References

Details

Attachments

(3 files, 1 obsolete file)

Attached image 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
tracking-fennec: --- → ?
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.
tracking-fennec: ? → +
Flags: needinfo?(margaret.leibovic)
Whiteboard: [mentor=margaret]
Filed Service Now request REQ0022273.
Assignee: nobody → margaret.leibovic
Flags: needinfo?(margaret.leibovic)
Whiteboard: [mentor=margaret]
Attached patch Patch (obsolete) — Splinter Review
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...
Attachment #8387455 - Flags: review?(lucasr.at.mozilla)
Comment on attachment 8387455 [details] [diff] [review]
Patch

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

Forgot to qref? :-)
Attachment #8387455 - Flags: review?(lucasr.at.mozilla)
Thanks for taking a look at this!
Assignee: margaret.leibovic → wjohnston
Attached patch PatchSplinter Review
Whoops.This should have code inside!
Attachment #8387455 - Attachment is obsolete: true
Attachment #8387665 - Flags: review?(lucasr.at.mozilla)
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?
Attachment #8387665 - Flags: review?(lucasr.at.mozilla)
Attached patch Alt PatchSplinter Review
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?
Attachment #8388765 - Flags: review?(lucasr.at.mozilla)
Flags: needinfo?(mark.finkle)
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.
Attachment #8388765 - Flags: review?(lucasr.at.mozilla) → review+
(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
Flags: needinfo?(mark.finkle)
https://hg.mozilla.org/mozilla-central/rev/e7f97fa8a86b
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Let's try to make sure bug 794581 is not regressed.
Depends on: 985400
(In reply to Mark Finkle (:mfinkle) from comment #13)
> Let's try to make sure bug 794581 is not regressed.

It did
You need to log in before you can comment on or make changes to this bug.