Closed Bug 769001 Opened 11 years ago Closed 11 years ago

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


(Firefox for Android Graveyard :: General, defect)

Not set


(firefox15 verified, firefox16 verified, firefox17 verified)

Firefox 16
Tracking Status
firefox15 --- verified
firefox16 --- verified
firefox17 --- verified


(Reporter: Margaret, Assigned: Margaret)



(2 files)

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.
Assignee: nobody → margaret.leibovic
Attached patch patchSplinter Review
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.
Attachment #639843 - Flags: review?(sriram)
Attached image 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 on attachment 639843 [details] [diff] [review]

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

This looks good to me.
(So the other patch for arrow isn't needed?)
Attachment #639843 - Flags: review?(sriram) → review+
(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.
Target Milestone: --- → Firefox 16
Comment on attachment 639843 [details] [diff] [review]

[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
Attachment #639843 - Flags: approval-mozilla-aurora?
Closed: 11 years ago
Resolution: --- → FIXED
Attachment #639843 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.