Closed Bug 867193 Opened 7 years ago Closed 7 years ago

Form autocomplete popup is positioned too high when the dynamic toolbar is visible

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 23
Tracking Status
firefox20 --- unaffected
firefox21 --- unaffected
firefox22 --- unaffected
firefox23 + verified
firefox24 + verified
fennec 23+ ---

People

(Reporter: yoanna_crisan, Assigned: cwiiis)

References

Details

(Keywords: regression, reproducible)

Attachments

(2 files)

Attached image Screenshot
Build: Firefox for Android 23.0a1(2013-04-30)
Device: Samsung Galaxy R
OS: Android 2.3.4

Steps to reproduce:
1.Go to gmail.com
2.Insert an username and press the "Sign in" button
3.Repeat step 2 for a new username
4.Delete the last inserted username 

Expected results:
- The prefilled usernames are displayed bellow the username field

Actual results:
- The prefilled usernames overlaps the username field and a little bit of the "Username" label
This is also reproducible using the latest Nightly on Nexus 4-(4.2.2) and Samsung Galaxy SII (2.3.4)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Hardware: x86_64 → ARM
tracking-fennec: --- → ?
Keywords: regression
Yoanna would you get a regression range using inbound builds if possible. http://ftp.mozilla.org/pub/mobile/tinderbox-builds/mozilla-inbound-android/
Summary: Prefilled usernames overlaps the username field on gmail.com → Prefilled usernames overlap the username field on gmail.com
Keywords: reproducible
The regression window for this issue is:

1. mozilla central
good build: 24.04.2013 
bad build:  25.04.2013 

pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=fef5f202b2dc&tochange=690b5e0f6562

2. inbound
good build: 1366819129
bad build: 1366819610

pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=93f79ae43c2a&tochange=684a5ca2efb7
Keywords: reproducible
Confirmed that this works with browser.chrome.dynamictoolbar; false
tracking-fennec: ? → 23+
Duplicate of this bug: 874247
In bug 874247, I mentioned fixing this in FormAssistPopup, but now I'm wondering if this is instead an issue with the return value of ElementTouchHelper.getBoundingContentRect(). There have been other issues with coordinates being incorrect in content (e.g. bug 872961), so maybe this fix needs to be closer to content.
Updating the summary to better reflect the exact issue.
Summary: Prefilled usernames overlap the username field on gmail.com → Form autocomplete popup is positioned too high when the dynamic toolbar is visible
Confirming that I only see this with the navigation bar visible on screen. On gmail, the moment I scroll the navigation bar off-screen the popup will drop correctly below the username field.
This takes the margin offset into account in FormAssistPopup.

This works, but if you make the toolbar hide/show while a popup is visible and don't cause it to re-show/hide, it will end up in the wrong position.

Perhaps a better way of fixing this would be to attach the layout that the popup is in to the bottom of the dynamic toolbar - now that I've thought of this, I prefer this method. I'll try to get to this, but feel free to take it.
Attachment #755344 - Flags: review?(margaret.leibovic)
(In reply to Chris Lord [:cwiiis] from comment #9)

> Perhaps a better way of fixing this would be to attach the layout that the
> popup is in to the bottom of the dynamic toolbar - now that I've thought of
> this, I prefer this method. I'll try to get to this, but feel free to take
> it.

I don't know if this will work well because the FormAssistPopup layout is in  shared_ui_components, which is designed to be used in web apps as well.
Comment on attachment 755344 [details] [diff] [review]
Take margin offset into account when positioning pop-up

This seems like a simple approach, although it would be good to write a follow-up patch to reposition the popup when the toolbar is shown/hidden. Is there a way to listen for that?
Attachment #755344 - Flags: review?(margaret.leibovic) → review+
Pushed to inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/f9424be46f38

Yes, we can listen to the metrics changing with onMetricsChanged (I think that's it - check GeckoLayerClient and BrowserApp, it listens for margin changes to move the toolbar)
https://hg.mozilla.org/mozilla-central/rev/f9424be46f38
Assignee: nobody → chrislord.net
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 24
Depends on: 877782
(In reply to Chris Lord [:cwiiis] from comment #12)
> Pushed to inbound:
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/f9424be46f38
> 
> Yes, we can listen to the metrics changing with onMetricsChanged (I think
> that's it - check GeckoLayerClient and BrowserApp, it listens for margin
> changes to move the toolbar)

Thanks for the pointer, I filed bug 877782 to implement this.
Verified fixed on:
Build: Firefox for Android 24.0a1 (2013-06-09)
Device: Samsung Galaxy R
OS: Android 2.3.4
Comment on attachment 755344 [details] [diff] [review]
Take margin offset into account when positioning pop-up

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 867193
User impact if declined: Auto-complete pop-ups obscure user input
Testing completed (on m-c, etc.): Been on m-c for a couple of weeks without complaint (afaik)
Risk to taking this patch (and alternatives if risky): Low risk
String or IDL/UUID changes made by this patch: None
Attachment #755344 - Flags: approval-mozilla-aurora?
Attachment #755344 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Pushed to aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/2158896cb923
Target Milestone: Firefox 24 → Firefox 23
Firefox for Android 23.0b2 (2013-07-02)
Device: LG Optimus 4x 
OS:Android 4.1.2
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.