Closed
Bug 764405
Opened 13 years ago
Closed 13 years ago
Misaligned door-hanger arrow with site-identity lock
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox15 verified, firefox16 verified, firefox17 verified)
VERIFIED
FIXED
Firefox 16
People
(Reporter: aaronmt, Assigned: Margaret)
References
Details
(Keywords: polish, regression)
Attachments
(3 files)
83.44 KB,
image/png
|
Details | |
2.43 KB,
patch
|
sriram
:
review+
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
1.08 KB,
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
See screenshot.
Assignee | ||
Comment 1•13 years ago
|
||
We hard-coded the position of that arrow, and it probably just needs to be updated to work with the new menu button.
Comment 2•13 years ago
|
||
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.
Assignee | ||
Comment 3•13 years ago
|
||
(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.
Comment 5•13 years ago
|
||
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.
Assignee | ||
Comment 7•13 years ago
|
||
(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 | ||
Updated•13 years ago
|
status-firefox15:
--- → affected
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → margaret.leibovic
Assignee | ||
Comment 8•13 years ago
|
||
This was a regression from: http://hg.mozilla.org/mozilla-central/rev/1330924c44f2
Blocks: 750707
Keywords: regression
Assignee | ||
Comment 9•13 years ago
|
||
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 10•13 years ago
|
||
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+
Assignee | ||
Comment 11•13 years ago
|
||
Target Milestone: --- → Firefox 16
Assignee | ||
Comment 12•13 years ago
|
||
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?
Comment 13•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Updated•13 years ago
|
status-firefox16:
affected → ---
Updated•13 years ago
|
Attachment #639842 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 14•13 years ago
|
||
Assignee | ||
Comment 15•13 years ago
|
||
Code in Aurora depends on this import.
Attachment #642248 -
Flags: review?(mark.finkle)
Assignee | ||
Comment 16•13 years ago
|
||
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)
Updated•13 years ago
|
Attachment #642248 -
Flags: review+
Reporter | ||
Updated•13 years ago
|
Updated•12 years ago
|
tracking-fennec: ? → ---
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•