Closed Bug 1020505 Opened 6 years ago Closed 6 years ago

Regression: Browser menu has width issues

Categories

(Firefox for Android :: General, defect)

31 Branch
All
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 35
Tracking Status
firefox31 --- unaffected
firefox32 --- unaffected
firefox33 --- unaffected
firefox34 + verified
firefox35 --- verified
fennec 34+ ---

People

(Reporter: rnewman, Assigned: lucasr)

References

Details

(Keywords: regression, reproducible)

Attachments

(2 files)

Note how Request Desktop Site's checkbox hangs off the right hand side.

Quickshare backout regression?
What device? Could be a layout XML issue for your DPI, blah, blah, blah.
HTC One M7, KitKat.
Oddly, on my Samsung Galaxy S5 (Android 4.4.4), I see this only on Nightly .. unless something triggers this. The checkbox on my S5 is even further pushed off-screen from the screenshot attached.
tracking-fennec: --- → ?
Summary: Main menu in 31 has width issues → Regression: Browser menu has width issues
Also see this on my Nexus 5 (Nightly (08/19)).

Mihai, can you help with a regression range here if you can reproduce?
Flags: needinfo?(mihai.g.pop)
2014-08-14 - good
2014-08-15 - bad
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5299864050ee&tochange=c9f8cc9ce89c
Flags: needinfo?(mihai.g.pop)
1408067296 - good
1408071493 - bad
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=f9c51223ffc9&tochange=842b44860ee2

I was only able to reproduce this issue on LG Nexus 4(Android 4.4.4).
Can you check fxteam?
1408045988 - good
1408047848 - bad
http://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=070d5351ed82&tochange=b9fc8a08a0c2

It seems that Bug 1042829 - Update Android SDK and build tools to 20 , caused this regression
Wes, any ideas?
Flags: needinfo?(wjohnston)
Assignee: nobody → lucasr.at.mozilla
tracking-fennec: ? → 34+
Clearing NEEDINFO. I'm taking a look at this.
Flags: needinfo?(wjohnston)
Status: NEW → ASSIGNED
Duplicate of this bug: 1058289
QA Contact: aaron.train
Duplicate of this bug: 1061633
Comment on attachment 8482845 [details] [diff] [review]
Don't truncate the menu popup width (r=margaret)

Not entirely sure what's causing the problem but removing the redundant MeasureSpec from the setWindowLayoutMode() call fixes. We set the width explicitly in menu_popup.xml already. Using WRAP_CONTENT is actually the right thing to do here. My guess is that PopupWindow doesn't deal very well with AT_MOST measure specs.

I can dive deeper into PopupWindow's source code but I'm happy to land this as an immediate fix.
Attachment #8482845 - Flags: review?(margaret.leibovic)
Careful. This was put in to fix bug 775272.
(In reply to Wesley Johnston (:wesj) from comment #15)
> Careful. This was put in to fix bug 775272.

It looks like that patch also added the explicit width in menu_popup.xml. I'm not sure why that wouldn't have been enough to fix the issue, but it sounds like it wasn't in bug 775272 comment 6.

Lucas, can you make sure this patch doesn't regress the fix from bug 775272?
Flags: needinfo?(lucasr.at.mozilla)
(In reply to :Margaret Leibovic from comment #16)
> (In reply to Wesley Johnston (:wesj) from comment #15)
> > Careful. This was put in to fix bug 775272.
> 
> It looks like that patch also added the explicit width in menu_popup.xml.
> I'm not sure why that wouldn't have been enough to fix the issue, but it
> sounds like it wasn't in bug 775272 comment 6.
> 
> Lucas, can you make sure this patch doesn't regress the fix from bug 775272?

Tested on a phone with L and a tablet with KitKat. Works fine. It would be nice if we could test this on devices with ICS and JB just to be safe.

Aaron, do you have tablets & phones running ICS and JB to this APK? Just want to make sure this patch doesn't regress the fix from bug 775272.

https://dl.dropboxusercontent.com/u/1187037/fennec-menu-fix.apk
Flags: needinfo?(lucasr.at.mozilla) → needinfo?(aaron.train)
I'll take a look, but Mihai, can you guys take a look too?
Flags: needinfo?(aaron.train) → needinfo?(mihai.g.pop)
It looks to be fixed, tested with Lucas build on:
LG Nexus 4(Android 4.4.4)
Alcatel One Touch(Android 4.1.2)
Samsung Galaxy Tab 2(Android 4.2.2)
LG Optimus 4X(Android 4.1.2)
Flags: needinfo?(mihai.g.pop)
Comment on attachment 8482845 [details] [diff] [review]
Don't truncate the menu popup width (r=margaret)

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

Thanks for testing. This change seems fine, maybe this part of the patch wasn't actually necessary for bug 775272.
Attachment #8482845 - Flags: review?(margaret.leibovic) → review+
https://hg.mozilla.org/mozilla-central/rev/182716b666bf
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Comment on attachment 8482845 [details] [diff] [review]
Don't truncate the menu popup width (r=margaret)

Approval Request Comment
[Feature/regressing bug #]: bug 1042829
[User impact if declined]: App menu looks broken.
[Describe test coverage new/current, TBPL]: Looks fine in Nightly.
[Risks and why]: Low, just changing the way a dialog is sized.
[String/UUID change made/needed]: n/a
Attachment #8482845 - Flags: approval-mozilla-aurora?
Status: RESOLVED → VERIFIED
Comment on attachment 8482845 [details] [diff] [review]
Don't truncate the menu popup width (r=margaret)

Aurora+
Attachment #8482845 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified as fixed in
Build: Firefox for Android 34.0a2 (2014-09-17)
Device: Nexus 4 Android 4.4.4
You need to log in before you can comment on or make changes to this bug.