Closed Bug 976680 Opened 6 years ago Closed 6 years ago

Promo banner on tablets in squished occasionally on rotation

Categories

(Firefox for Android :: General, defect)

30 Branch
ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 31
Tracking Status
firefox29 --- verified
firefox30 --- verified
firefox31 --- verified
fennec 29+ ---

People

(Reporter: aaronmt, Assigned: Margaret)

References

Details

(Keywords: reproducible)

Attachments

(2 files)

Attached image screenshot.png
See screenshot
tracking-fennec: ? → 29+
Assignee: nobody → margaret.leibovic
Aaron, which tablet were you using here? I'm not seeing this on my N7.
NI for comment 1
Flags: needinfo?(aaron.train)
Was using my Nexus 7 2013 (Android 4.4.2). I believe I hit this scenario occasionally after rotating the device on about:home. As you can see in the screenshot it looks like the logic got confused on the width for some reason and when it was drawn it was all squished.
Flags: needinfo?(aaron.train)
Summary: Promo banner on tablets in squished → Promo banner on tablets in squished occasionally on rotation
I'll hunt for specific steps on Monday
Flags: needinfo?(aaron.train)
Ah steps are trivial to reproduce actually

i) Hold the tablet in landscape and launch Nightly
ii) Rotate to portrait before the banner is shown on about:home
Flags: needinfo?(aaron.train)
This is tracking Fx29 folks. What's the plan?
(In reply to Mark Finkle (:mfinkle) from comment #6)
> This is tracking Fx29 folks. What's the plan?

The plan is I still need to look into a fix, but I won't be able to get to this until next week.

I think this is enough of an edge case that it's not the end of the world if we ship this bug, but it would be good to fix it.
Status: NEW → ASSIGNED
I found that this issue is caused by the fact that we have different padding values for portrait/landscape tablet views, and the padding isn't refreshed when the view is rotated.

An easy fix here would be to just get rid of that padding, or to make it the same value for both portrait and landscape. I've actually found it a bit awkward that the close button isn't all the way on the right in landscape mode on tablets, so maybe this would be a good fix.

Ian, what do you think?
Flags: needinfo?(ibarlow)
(In reply to :Margaret Leibovic from comment #8)
> I found that this issue is caused by the fact that we have different padding
> values for portrait/landscape tablet views, and the padding isn't refreshed
> when the view is rotated.
> 
> An easy fix here would be to just get rid of that padding, or to make it the
> same value for both portrait and landscape. I've actually found it a bit
> awkward that the close button isn't all the way on the right in landscape
> mode on tablets, so maybe this would be a good fix.
> 
> Ian, what do you think?

I agree. Let's use the same padding for both :)
Flags: needinfo?(ibarlow)
I kept the Widget.HomeBanner style because we're still using that to set some padding for a non-landscape tablet style.
Attachment #8397492 - Flags: review?(bnicholson)
Attachment #8397492 - Flags: review?(bnicholson) → review+
Comment on attachment 8397492 [details] [diff] [review]
Remove landscape padding from home banner

[Approval Request Comment]
Bug caused by (feature/regressing bug #): enabling snippets/sync promo banner
User impact if declined: banner contents look squished if a tablet is rotated
Testing completed (on m-c, etc.): just landed on fx-team
Risk to taking this patch (and alternatives if risky): low-risk, removes some padding in our landscape tablet UI
String or IDL/UUID changes made by this patch: none
Attachment #8397492 - Flags: approval-mozilla-beta?
Attachment #8397492 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/07b7da9bf7cc
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 31
Attachment #8397492 - Flags: approval-mozilla-beta?
Attachment #8397492 - Flags: approval-mozilla-beta+
Attachment #8397492 - Flags: approval-mozilla-aurora?
Attachment #8397492 - Flags: approval-mozilla-aurora+
Verified as fixed in builds:
- 31.0a1 (2014-04-07);
- 30.0a2 (2014-04-07);
Device: Google Nexus 7 2013 (Android 4.4.2).

Will retest on beta 6 tomorrow after the build is landed.
Verified as fixed in build Firefox 29 beta 6;
Device: Google Nexus 7 2013 (Android 4.4.2).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.