769001 - [tablet] Site identity popup takes up the entire screen width

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[:Margaret Leibovic]

The issue from bug 716095 also exists for the site identity popup. If we create a way to dynamically position the arrow/popup, we could share a generic arrow panel layout for the doorhanger popup and the site identity popup, and this would also fix bug 764405 and bug 768073.

Comment 1

[:Margaret Leibovic]

Attachment: patch

Do we have any strategies for avoiding so much duplicate layout code? I only had to change the layout_width on the inner RelativeLayout to get this to work. I tried dynamically doing that in the SiteIdentityPopup constructor, but that wasn't working right because I had to create new LayoutParams, and they didn't account for the top margin that's in the XML. I also know we prefer doing things in XML for performance reasons, but I get worried about these files accidentally getting out of sync.

Comment 2

[:Margaret Leibovic]

Attachment: screenshot

This has the patch from bug 739364 backed out to fix bug 771380 locally. Without the backout, the popup just appears lower than it should because of the extra padding on the icon.

Comment 3

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 639843 [details] [diff] [review]
patch

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

This looks good to me.
(So the other patch for arrow isn't needed?)

Comment 4

[:Margaret Leibovic]

(In reply to Sriram Ramasubramanian [:sriram] from comment #3)

> This looks good to me.
> (So the other patch for arrow isn't needed?)

Do you mean bug 764405? That's still needed for the arrow to align properly on phones, since we're keeping the same logic there to make the popup take up the entire screen width.

Comment 5

[:Margaret Leibovic]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d99fa5ee8bd3

Comment 6

[:Margaret Leibovic]

Comment on attachment 639843 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): polish issue with tablet UI, which is new to 15
User impact if declined: site identity popup is too wide
Testing completed (on m-c, etc.): just landed on inbound
Risk to taking this patch (and alternatives if risky): low-risk style change
String or UUID changes made by this patch: n/a

Comment 7

[Ryan VanderMeulen [:RyanVM]]

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

Comment 8

[:Margaret Leibovic]

https://hg.mozilla.org/releases/mozilla-aurora/rev/22dba548db57