Regression: Browser menu has width issues

VERIFIED FIXED in Firefox 34

Status

()

Firefox for Android
General
VERIFIED FIXED
4 years ago
a year ago

People

(Reporter: rnewman, Assigned: lucasr)

Tracking

({regression, reproducible})

31 Branch
Firefox 35
All
Android
regression, reproducible
Points:
---

Firefox Tracking Flags

(firefox31 unaffected, firefox32 unaffected, firefox33 unaffected, firefox34+ verified, firefox35 verified, fennec34+)

Details

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
Created attachment 8434332 [details]
Screenshot_2014-06-04-11-16-18.png

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

Comment 2

4 years ago
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: --- → ?
status-firefox32: unaffected → ?
status-firefox33: --- → ?
status-firefox34: --- → affected
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)
Keywords: regression, regressionwindow-wanted, reproducible

Comment 5

3 years ago
2014-08-14 - good
2014-08-15 - bad
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5299864050ee&tochange=c9f8cc9ce89c
Flags: needinfo?(mihai.g.pop)

Comment 6

3 years ago
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).
Keywords: regressionwindow-wanted
Can you check fxteam?

Comment 8

3 years ago
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
(Assignee)

Comment 9

3 years ago
Wes, any ideas?
Flags: needinfo?(wjohnston)
Assignee: nobody → lucasr.at.mozilla
tracking-fennec: ? → 34+
status-firefox31: affected → ?
(Assignee)

Comment 10

3 years ago
Clearing NEEDINFO. I'm taking a look at this.
Flags: needinfo?(wjohnston)

Updated

3 years ago
Status: NEW → ASSIGNED

Updated

3 years ago
Duplicate of this bug: 1058289

Updated

3 years ago
QA Contact: aaron.train

Updated

3 years ago
Duplicate of this bug: 1061633
(Assignee)

Comment 13

3 years ago
Created attachment 8482845 [details] [diff] [review]
Don't truncate the menu popup width (r=margaret)
(Assignee)

Comment 14

3 years ago
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.

Comment 16

3 years ago
(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)
(Assignee)

Comment 17

3 years ago
(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)

Comment 19

3 years ago
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 20

3 years ago
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+
(Assignee)

Comment 21

3 years ago
https://hg.mozilla.org/integration/fx-team/rev/182716b666bf
https://hg.mozilla.org/mozilla-central/rev/182716b666bf
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
(Assignee)

Comment 23

3 years ago
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?

Updated

3 years ago
Status: RESOLVED → VERIFIED
status-firefox35: --- → verified
status-firefox31: ? → unaffected
status-firefox32: ? → unaffected
status-firefox33: ? → unaffected
tracking-firefox34: --- → +
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+
https://hg.mozilla.org/releases/mozilla-aurora/rev/99043f498353
status-firefox34: affected → fixed
Verified as fixed in
Build: Firefox for Android 34.0a2 (2014-09-17)
Device: Nexus 4 Android 4.4.4
status-firefox34: fixed → verified
You need to log in before you can comment on or make changes to this bug.