Closed Bug 872528 Opened 6 years ago Closed 6 years ago

Password doorhanger is not displayed on Gingerbread

Categories

(Firefox for Android :: General, defect)

24 Branch
ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 26
Tracking Status
firefox23 --- wontfix
firefox24 --- wontfix
firefox25 --- verified
firefox26 --- verified
fennec 23+ ---

People

(Reporter: TeoVermesan, Assigned: sriram)

References

Details

(Keywords: regression, reproducible)

Attachments

(1 file)

Tested with:
Build: Firefox for Android 24.0a1 (2013-05-15)
Device: Samsung Galaxy R
OS: Android 2.3

Steps to reproduce:
1.Go to gmail.com
2.Insert username and password 
3.Tap "Sign in" 

Expected results:
- A password doorhanger is displayed to save/don't save the password

Actual results:
- The password doorhanger is not displayed
Works for me. What do you see when you clear cookies and active logins, site settings and cache? Reload Google and try again.
In can reproduce this on HTC Desire Z (Android 2.3)
Please see attached video: https://www.youtube.com/watch?v=YaGvexW9D9I
Summary: Password doorhanger is not displayed on Gmail → Password doorhanger is not displayed on gmail.com on Gingerbread
This is reproducible on Aurora 25.0a2 2013-08-08 and Nightly 26.0a1 2013-08-08 on the HTC Desire HD (Android 2.3.5) on gmail.com, facebook,com and twitter.com.
tracking-fennec: --- → ?
Summary: Password doorhanger is not displayed on gmail.com on Gingerbread → Password doorhanger is not displayed on Gingerbread
Did this work in 23? Is it a regression?
I can reproduce this on Firefox for Android 23 RC. 
- If the URL bar is hidden, then the password doorhanger doesn't appear.
- If I disable the dynamic toolbar, the password doorhanger appears.
Note: The steps are the same from comment 7 from Bug 858483.
Assignee: nobody → margaret.leibovic
tracking-fennec: ? → 23+
I started investigating this yesterday, and I found that we do correctly detect that the anchor is out of view, but for some reason the popup doesn't appear when we call showAtLocation here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/widget/ArrowPopup.java#80

Sriram, do you have any idea what may be different about PopupWindow on 2.3 that causes this to fail?
Flags: needinfo?(sriram)
I dont know why this happens in GB alone. I tried setting a value like 50 for y, so that it accounts for the system ui bar. Still I didn't show up. showAtLocation() uses the view passed in to get the window token alone. So, its not dependent on GeckoLayout. Also the IBinder of the mActivity.getView() is alive too (as expected).
Flags: needinfo?(sriram)
Moving to Sriram
Assignee: margaret.leibovic → sriram
Actually the popup is shown, but it's width and height are 0. This is because of an Android bug:

https://github.com/android/platform_frameworks_base/blame/master/core/java/android/widget/PopupWindow.java#L845

We need to way to push those two lines through our call (which doesnt feel like possible). We need to set a default width and height. <-- This may vary.

/me still trying to figure out how to hack this.
Oh.. the actual bug is, those two lines are not available in GB.
Attached patch PatchSplinter Review
So this is the best approach I can think of. Basically setting as much width and height for the popup. And the popup also dismisses when touched outside the actual "content" shown.

Note: This doesn't work with rotation. Because, we don't handle rotation properly :P The size doesn't alter and hence would look wierd.

But what's the probability of someone using a GB phone, with browser-toolbar hidden, enters a password, a doorhanger shows up and rotates the phone before selecting an option in the doorhanger?
Attachment #800600 - Flags: review?(margaret.leibovic)
Comment on attachment 800600 [details] [diff] [review]
Patch

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

This looks good to me, but I wasn't able to test it because I couldn't get the popup to appear on gmail.com anymore (also happens on release, maybe something to do with them changing their site?).
Attachment #800600 - Flags: review?(margaret.leibovic) → review+
Will consider a low risk uplift for aurora, but no need to track as we would not block Fx24 for this issue.
Sriram - Let's get this landed and get an uplift for Aurora
Flags: needinfo?(sriram)
https://hg.mozilla.org/mozilla-central/rev/4d0f4b412d32
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
Comment on attachment 800600 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Dynamic toolbars
User impact if declined: Password doorhangers won't be shown if the dynamic toolbar is hidden.
Testing completed (on m-c, etc.): Landed in m-c on 09/09.
Risk to taking this patch (and alternatives if risky): Very very low. We set a default size for the popups.
String or IDL/UUID changes made by this patch: Zilch.
Attachment #800600 - Flags: approval-mozilla-beta?
Attachment #800600 - Flags: approval-mozilla-aurora?
Comment on attachment 800600 [details] [diff] [review]
Patch

Too late for fx24 given where we are in the cycle.Approving for aurora only at this time.
Attachment #800600 - Flags: approval-mozilla-beta?
Attachment #800600 - Flags: approval-mozilla-beta-
Attachment #800600 - Flags: approval-mozilla-aurora?
Attachment #800600 - Flags: approval-mozilla-aurora+
Verified fixed on:
Build: Firefox for Android 26.0a1 (2013-09-12) and Firefox for Android 25.0a2 (2013-09-12)
Device: Samsung Galaxy R
OS: Android 2.3
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.