Closed Bug 764405 Opened 9 years ago Closed 9 years ago

Misaligned door-hanger arrow with site-identity lock

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

(firefox15 verified, firefox16 verified, firefox17 verified)

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

People

(Reporter: aaronmt, Assigned: Margaret)

References

Details

(Keywords: polish, regression)

Attachments

(3 files)

See screenshot.
We hard-coded the position of that arrow, and it probably just needs to be updated to work with the new menu button.
I've been thinking of moving DoorHanger to BrowserToolbar (as an interface), so that a layout / size changed in BrowserToolbar (like when tabs are shown), we can resize/reposition DoorHanger.
(In reply to Sriram Ramasubramanian [:sriram] from comment #2)
> I've been thinking of moving DoorHanger to BrowserToolbar (as an interface),
> so that a layout / size changed in BrowserToolbar (like when tabs are
> shown), we can resize/reposition DoorHanger.

That definitely sounds like a better solution. SiteIdentityPopup uses its own layouts, so we'd also have to change that, or maybe we can make a shared arrow panel that we can position and fill up with different contents.
Duplicate of this bug: 764737
Don't know if it is worth a new bug so I just add it here for now: same problem also shows with the menu arrow pointing to the menu button, it's pointing to the left side of the icon instead of the center.
Duplicate of this bug: 767129
(In reply to Michael Monreal [:monreal] from comment #5)
> Don't know if it is worth a new bug so I just add it here for now: same
> problem also shows with the menu arrow pointing to the menu button, it's
> pointing to the left side of the icon instead of the center.

Bug 768043 was filed about this.
Assignee: nobody → margaret.leibovic
This was a regression from: http://hg.mozilla.org/mozilla-central/rev/1330924c44f2
Blocks: 750707
Keywords: regression
Attached patch patchSplinter Review
The margin moved from the ImageButton itself to its parent layout in lucasr's patch.

It feels fragile to be computing the margin in this way, but this fixes the immediate problem.
Attachment #639842 - Flags: review?(sriram)
Comment on attachment 639842 [details] [diff] [review]
patch

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

This looks good to me. We should find a better way to make all these robust.
I am thinking of some onSizeChangedListeners for the BrowserToolbar, which these PopupWindows can listen and update the icons.
This would be fine, the PopupWindow on the left will be affected with the shrinking/expanding of BrowserToolbar during showing/hiding TabsTray.
Attachment #639842 - Flags: review?(sriram) → review+
Comment on attachment 639842 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): regression caused by bug 750707
User impact if declined: site identity popup is misaligned on phones
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 #639842 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/6170836e4f9e
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Attachment #639842 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Code in Aurora depends on this import.
Attachment #642248 - Flags: review?(mark.finkle)
Comment on attachment 642248 [details] [diff] [review]
add back import on aurora

I just pushed this to fix the bustage:
https://hg.mozilla.org/releases/mozilla-aurora/rev/67d61a9353d3
Attachment #642248 - Flags: review?(mark.finkle)
Status: RESOLVED → VERIFIED
tracking-fennec: ? → ---
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.