808248 - Tweak doorhanger arrow positioning

Status: RESOLVED DUPLICATE of bug 847578

(Firefox for Android Graveyard :: General, defect)

Description

[:Margaret Leibovic]

Attachment: phone screenshot

We've fixed a bunch of bugs to try to make the doorhanger arrows point to the right place, but I think we could use another pass to make this positioning better.

Here are the issues I see:

1) On phones, the doorhanger popup is positioned too low, and the arrow is a bit too far to the right (see attachment).
2) If the phone is rotated, the vertical positioning of the popup is not adjusted to account for the different toolbar size.
3) On tablets, the popup is also positioned too low, but the arrow is a bit too far to the right.

I'm marking this as a mentor bug, but it may be difficult for a contributor who doesn't own both a phone and a tablet.

Sriram, I'm cc'ing you because you may be interested in this, or interested in helping someone trying to work on it :)

Comment 1

[:Margaret Leibovic]

Attachment: tablet screenshot

Comment 2

[Sriram Ramasubramanian [:sriram]]

Ian is working on new doorhanger design, and I'm waiting for that. This might all change with that.

Comment 3

[:Margaret Leibovic]

Okay, I'll stay tuned, then. For now, it's good to have these issues on file.

Comment 4

[Andreea Pod]

Attachment: security doorhanger

This is still reproducible on Firefox for Android 18 Beta 5 (2012-11-06) on ASUS EEE Transformer (Android 4.0.3) please see the screenshot.

Comment 5

[vchinen]

Hello Margaret, is this a workable bug? If so, I'll be interested to work on it. Thank you.

Comment 6

[Sriram Ramasubramanian [:sriram]]

Hi vchinen, I can help you in this.

Comment 7

[Jenifer Wang]

Hi! I'll be working on this with vchinen. I'm very new here and I would like to know if building on Fennec is the right procedure to tackle this. If it is, how can I reproduce 'attachment 677954 [details]' on Fennec?

Additional note to the bug: The position of the doorhanger is to the right and not the correct position if tabs are on.

Comment 8

[:Margaret Leibovic]

Sorry I've been slow to respond here, I've been traveling. Sriram is probably a better person that me to help out anyway, since the code has probably changed some since this bug was filed.

Building fennec is definitely the right place to start. Have you seen this wiki page?
https://wiki.mozilla.org/Mobile/Fennec/Android#Building_Fennec

After building, you should see if you can reproduce the issue. The code for this popup lives in DoorHangerPopup.java, so you can look around there to figure out how things work.

Also, feel free to join us on IRC for faster turnaround time answering questions. The people working on fennec hang out in #mobile on irc.mozilla.org.

Comment 9

[vchinen]

Hello. Jenifer and I have some questions about fixing the bug. 

We are reading the code from 
https://hg.mozilla.org/mozilla-central/file/tip/mobile/android/base/DoorHangerPopup.java#l272
and we understand that we need to make the arrow point at the center of the anchor. However, according to the comments in the code, this has already been done. If so, what would be the next step to fix the bug? 

Thank you!

Comment 10

[:Margaret Leibovic]

(In reply to vchinen from comment #9)

> We are reading the code from 
> https://hg.mozilla.org/mozilla-central/file/tip/mobile/android/base/
> DoorHangerPopup.java#l272
> and we understand that we need to make the arrow point at the center of the
> anchor. However, according to the comments in the code, this has already
> been done.

The comments say that, but if it's not happening, then that means the code isn't working :)

As the comment mentions, on phones, the arrow positioning is actually based on the left margin of the arrow, so you may want to play around with modifying that:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/layout/doorhangerpopup.xml#26

Sriram may be able to offer more advice, but I think you should start by getting comfortable understanding how this code works, and a great way to do that is to start making some changes and see what happens.

Comment 11

[Stefan :Sobering]

Is this something I could take on? First time contributor.

Comment 12

[:Margaret Leibovic]

(In reply to ss from comment #11)
> Is this something I could take on? First time contributor.

Welcome! Unfortunately, it looks like Sriram fixed this issue in bug 847578.

However, there are lots of other bugs that could use fixing. Once you get a build environment set up (https://wiki.mozilla.org/Mobile/Fennec/Android), you should take a look at some other mentor bugs (http://www.joshmatthews.net/bugsahoy/?mobile=1). Definitely join #mobile on irc.mozilla.org if you need help!