LastPass, 1Password, Bitwarden and all 3rd-party password manager auto-fill is broken in GeckoView

RESOLVED FIXED in Firefox 63

Status

defect
P1
major
RESOLVED FIXED
3 years ago
2 months ago

People

(Reporter: cwiiis, Assigned: jchen)

Tracking

(Blocks 3 bugs)

50 Branch
mozilla63
ARM
Android
Dependency tree / graph

Firefox Tracking Flags

(fennec+, firefox-esr52 wontfix, firefox-esr60 wontfix, firefox61 wontfix, firefox62 wontfix, firefox63 fixed)

Details

(Whiteboard: [geckoview:klar:p1])

Attachments

(13 attachments)

4.88 KB, text/plain
Details
137.09 KB, image/png
Details
1.82 MB, video/mp4
Details
2.53 MB, video/mp4
Details
477.52 KB, image/png
Details
46 bytes, text/x-phabricator-request
eeejay
: review+
Details | Review
46 bytes, text/x-phabricator-request
droeh
: review+
Details | Review
46 bytes, text/x-phabricator-request
droeh
: review+
Details | Review
46 bytes, text/x-phabricator-request
esawin
: review+
Details | Review
46 bytes, text/x-phabricator-request
eeejay
: review+
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
esawin
: review+
snorp
: review+
Details | Review
46 bytes, text/x-phabricator-request
snorp
: review+
Details | Review
Reporter

Description

3 years ago
In Firefox 49, LastPass worked with Firefox for Android (albeit in a limited fashion, requiring you to copy/paste usernames/passwords, as opposed to them actually being filled in like with Chrome and other Android applications).

In Firefox 50, LastPass doesn't work at all. If you tap the LastPass notification, it just displays a toast that reads 'LastPass was unable to auto-fill this app'.

Putting this in Keyboards/IME but uncertain of the mechanism LastPass actually uses to auto-fill applications.
tracking-fennec: --- → ?
LastPass is #9 of Top10 Android Add-ons, so we should get this verified and fixed soon.

NI Sebastian/Joe
Flags: needinfo?(s.kaspari)
Flags: needinfo?(jcheng)
Yeah, let's talk about this at triage tomorrow morning. We just missed the Firefox 51 release unfortunately.
Flags: needinfo?(s.kaspari)
Assignee: nobody → topwu.tw
tracking-fennec: ? → +
Priority: -- → P1
Hi Sorina,
could you provide a regression window?
Flags: needinfo?(sorina.florean)
I've tried Firefox 49 with latest LastPass, but autofill still doesn't work. It still shows 'LastPass was unable to auto-fill this app'.

I sent a support ticket(https://lastpass.com/my.php?token=U3ZGKV4WUCZF) to LastPass to clarify if Fennec broke autofill function or LastPass doesn't support us to fill credential through notification at all.

Btw in Fennec we can still use LastPass through Addons(https://addons.mozilla.org/en-US/android/addon/lastpass-password-manager/?src=search)
The response from LastPass:

> The LastPass app is not optimized to work in Firefox - we're sorry about that. 
> Only the information provided re the add-on can be used to autofill site entries in that browser on mobile.
Reporter

Comment 6

2 years ago
Well, this definitely used to work via the LastPass Android application and definitely doesn't work now - perhaps due to an update on their side (though maybe worth trying 48 in case I got the numbers mixed up?)

Worth noting that the Firefox add-on doesn't work with free accounts and also requires separate log-in via modal dialog (which is pretty cumbersome if you have 2FA and rely on your phone for 2FA...)
I've tried 48 but LastPass notification still doesn't work. 

Also I can use add-on with free account which is still in trial period.
Reporter

Comment 8

2 years ago
(In reply to Jing-wei Wu [:jwu] from comment #7)
> I've tried 48 but LastPass notification still doesn't work. 

I guess LastPass must've changed something in the app that broke Firefox in that case.

It would be good if we could figure out how it works in Chrome (it also works in Brave, but that appears to be a fork of Chromium anyway) and replicate whatever annotation is necessary to have it work in Firefox.

> Also I can use add-on with free account which is still in trial period.

Yes, once the trial period is over, the add-on no longer works.
clear the NI on me as the bug already has P1 Priority with tracking-fennec+
Flags: needinfo?(jcheng)
Mike do we have any business contacts with LastPass? AMO team or mconnor might know more.
Flags: needinfo?(mozilla)
I'm pretty sure we do. Copying kev who would know.
Flags: needinfo?(mozilla) → needinfo?(kev)
Jing-wei, are you still working on possible workaround in Fennec? Please deassign if you are no longer working on that (any maybe post some finding if you have any).
Flags: needinfo?(topwu.tw)
Bill, I don't know if you're still talking with the LP team, but if you are, could you bring this up?
Flags: needinfo?(kev)

Updated

2 years ago
Assignee: topwu.tw → nobody
Flags: needinfo?(topwu.tw)
Reporter

Comment 14

2 years ago
Adding n?billm to answer comment #13. LastPass is still broken on Android, fwiw.
Flags: needinfo?(wmccloskey)
Andrew might be able to help.
Flags: needinfo?(wmccloskey) → needinfo?(drew)

Comment 16

2 years ago
anatoly@lastpass.com would probably be the best person to triage this.
Flags: needinfo?(drew)
Thanks Andrew. Can you help with this issue Anatoly?
Flags: needinfo?(anatoly)

Comment 18

2 years ago
Just to be clear, this is not an issue with our Firefox extension for Android, but rather the standalone app. Our app depends on Android's accessibility API to look for the edit or text object that is the URL bar, and read its contents to determine the current URL. It looks like in the latest versions of Firefox, this is no longer possible, as the URL bar is no longer a discrete object that is reported by accessibility, as confirmed with uiautomatorviewer. Therefore, our app is unable to determine the current URL, and gives up after a short timeout with the "unable to auto-fill" error.

If you can provide us another method to read the current URL in Firefox, we can restore this functionality. We have one option that we have used with a couple other third-party browsers, and I would be happy to discuss that in email.
Flags: needinfo?(anatoly) → needinfo?(wmccloskey)
I'm not sure who to needinfo about this. Snorp, can you redirect as needed based on comment 18?
Flags: needinfo?(wmccloskey) → needinfo?(snorp)
urlbar issue, so I think the frontend folks should be able to help here. Tim, can you respond to comment #18?

-> timdream
Flags: needinfo?(snorp) → needinfo?(timdream)
Max, do you have enough information here to work on this? Do we need a test to make sure accessibility won't regress?
Flags: needinfo?(timdream) → needinfo?(max)

Comment 22

2 years ago
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #21)
> Max, do you have enough information here to work on this? Do we need a test
> to make sure accessibility won't regress?

I can contact Anatoly to find a solution for this bug.

@Tim
As for you mentioned about testing to prevent regression, we need to define an interface for testing. So I wonder which approach is more appropriate:
1. Firefox provides an SDK/API for 3rd party(LassPass) to retrieve the URL
2. LastPass provide the SDK/API for Firefox to implement to provide the URL

Since Google also provide password management from SmartLock and new Auto-Fill from "O" release, I guess we might already have some discussion on this topic.

@Joe, What do you think?
Assignee: nobody → max
Flags: needinfo?(timdream)
Flags: needinfo?(max)
Flags: needinfo?(jcheng)
(In reply to Anatoly from comment #18)
> Our app depends on Android's
> accessibility API to look for the edit or text object that is the URL bar,
> and read its contents to determine the current URL. It looks like in the
> latest versions of Firefox, this is no longer possible, as the URL bar is no
> longer a discrete object that is reported by accessibility, as confirmed
> with uiautomatorviewer.

It seems like a change in Firefox that's breaking this feature? can we look into it?
Flags: needinfo?(jcheng)
Yeah it looks like as long as our UI can be accessed by accessibility API we should be good, no?
Flags: needinfo?(timdream)

Comment 25

2 years ago
Yes, that's all we would need to restore the previous functionality. Of course support for filling forms would be even better, but that's a separate issue. :) Also please keep in mind that there will be auto-fill changes in Android O, since Google is developing the auto-fill framework. (https://developer.android.com/preview/features/autofill.html)
yup, looking at the Android O auto-fill feature as well
Reporter

Comment 27

2 years ago
Been another month, sorry for the spam but wanted to check if there's any progress on this?
We've visited this bug again on sprint planning. Unfortunately, we will not be able to address this yet because of other bugs and features.
Reporter

Comment 29

2 years ago
Is there any more chance now of getting this looked at? This is an awfully long time for a top-tier product to remain incompatible. Is the work that's needed to fix this issue extensive?
Flags: needinfo?(timdream)
Unfortunately, I am no longer being assiged to Fennec. It's unclear if we will get to this.
Flags: needinfo?(timdream)
(In reply to Anatoly from comment #18)
> Just to be clear, this is not an issue with our Firefox extension for
> Android, but rather the standalone app. Our app depends on Android's
> accessibility API to look for the edit or text object that is the URL bar,
> and read its contents to determine the current URL. It looks like in the
> latest versions of Firefox, this is no longer possible, as the URL bar is no
> longer a discrete object that is reported by accessibility, as confirmed
> with uiautomatorviewer. Therefore, our app is unable to determine the
> current URL, and gives up after a short timeout with the "unable to
> auto-fill" error.
> 
> If you can provide us another method to read the current URL in Firefox, we
> can restore this functionality. We have one option that we have used with a
> couple other third-party browsers, and I would be happy to discuss that in
> email.

eeejay, can you comment on what is _supposed_ to happen for a11y of the urlbar?  That is, can we "just" try to fix this bug by addressing the a11y element layout of the urlbar in Fennec?  (If you're the wrong person, can you redirect?)

That would be a lot faster and probably easier than:

1) an SDK for Web Extensions
2) building some Android O extension mechanism
Flags: needinfo?(eitan)

Updated

Last year
Assignee: max → nobody
[triage] Technically this issue is keyboards/IME but if we move it to my component, we might get some resources - setting as P1 critical issue for now.
Component: Keyboards and IME → General
This really would be an abuse of the a11y API. There is a form autofill framework that is designed for this kind of use. Nonetheless, I'll attach a jsonified accessible tree of fennec. The current URL can be scraped from the TextView..
Flags: needinfo?(eitan)

Comment 35

Last year
Mike I would keep this as a typical P1, not a critical one.
Flags: needinfo?(sdaswani)

Updated

Last year
Whiteboard: [Leanplum] [61]
Reporter

Comment 36

Last year
Please note that all Android third party password managers are broken in Firefox and not just LastPass - it'd be great to have bitwarden working as well (and given that's open source, perhaps an easier task to debug).

Updated

Last year
Severity: normal → major
Did some investigations regarding this bugs and found the following:

- Firefox does expose the url so that other services could use that address as Anatoly said we need to do [1] - attachment 8974062 [details]

- In latest release versions - 59.0.2 and 60.0 the login autofill works with LastPass as described in the first comment - attachment 8974072 [details]
> albeit in a limited fashion, requiring you to copy/paste usernames/passwords, as opposed to them actually being filled in like with Chrome and other Android applications

- On latest dev build - 62.0.a1 LastPass can't correctly identify the site for which it should show password auto-fill suggestions - attachment 8974073 [details]

- On other intermediary builds (from 60 to 62) LastPass did nothing, not even present that Toast saying it can't autofill the current app.

Asked for help to our QA colleagues to try to get a regression range and identify in which builds this feature works or not, in which capacity, because the behavior seems to be quite random atm.

Also I don't exactly know how LastPass could interact with Gecko for the autofill, will look into that.


[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1330257#c18
Wasn't able to find a regression range on Nightly builds because in app settings (Settings-> Autofill ->app settings -> Nightly) when I search for browser I've got "do not show fill window".

 However tested on releases build starting with 59.0.2 till 40.0 and the fill window is displayed if I press on LastPass notification and I can copy/paste the password and username. So it seems that only on release is working just like in description.
Flags: needinfo?(sorina.florean)

Comment 42

Last year
Hi, I am a Bitwarden developer. I figured I would post some info on how Bitwarden works for autofilling through our accessibility service:

1. We look for some text field (by id) on the screen that has the URL of the current website. We have these hard-coded into our accessibility service for each supported browser. ex:

https://github.com/bitwarden/mobile/blob/master/src/Android/AutofillService.cs#L47
https://github.com/bitwarden/mobile/blob/master/src/Android/AutofillService.cs#L311

2. We parse the AccessibilityNodeInfo tree looking for fields of type Password. We then locate the field prior to the password and assume it is the username. ex:

https://github.com/bitwarden/mobile/blob/master/src/Android/AutofillService.cs#L442

Note that the above code is C# written in Xamarin rather than normal Android Java.

P.S. This is for autofilling using our accessibility service. We would also love to support autofill using the Oreo Autofill Framework too, but Firefox needs to add support there too.
I investigated this more with the help of Sorina and Kyle and to conclude my findings:

* The autofill feature using 3rd party services may have never worked fully (Sorina confirmed even 40 didn't offer this)
In the recent versions, from our tests on release versions the feature works as described in comment 0
> in a limited fashion, requiring you to copy/paste usernames/passwords, as opposed to them actually being filled in like with Chrome and other Android applications

* Being that LastPass keeps itself always in the notification bar it can be forced to identify an url in the running app and try to offer login data, but other password manager apps that do not stay there will not be able to offer such data.

* LastPass manages to show the dialog from which the user can copy the username and password as the url is made available from the app - attachment 8974062 [details]

* As confirmed with Kyle our app should also expose web contents structure to the accessibility layer where form inputs for username and password could be found. 
This would let password manager apps traverse the current view hierarchy and access the username and password fields as AccessibilityNodeInfo objects and then offer to autofill those fields. 
I think this happens on the WebView that Focus uses but not with our own Gecko/GeckoView - attachment 8974386 [details]
and this should be where we should focus on to complete the feature.

I'll make another ticket for supporting autofill using the new Oreo Autofill Framework as I think that would require other changes.
NI'ing James to see if he thinks the suggested approach to expose web contents structure is doable.
Flags: needinfo?(snorp)

Comment 46

Last year
Yes, I can confirm that we have no issues autofilling in Focus. Accessibility and Oreo both work there using Bitwarden.
Fully exposing web content to a11y is a big job, but one that Eitan has been looking into. I don't think we have plans to do this in the short-term, but maybe he'll have something to add here.
Flags: needinfo?(snorp) → needinfo?(eitan)

Comment 48

Last year
Andreas and Barbara, can you prioritize this work for us? Is this something that must be fixed? And if so, by when?
Flags: needinfo?(bbermes)
Flags: needinfo?(abovens)
Reporter

Comment 49

Last year
As a quicker fix, could just user+password fields be exposed to a11y as opposed to the entire page? I don't know if that's actually any easier, but personally I think this bug is absolutely critical. I've not used Firefox in over a year because of it and I've stopped recommending it to people because I can't in good conscience do so when using the browser means compromising on security. Firefox accounts offers no 2FA and no external app integration, so it really isn't a good choice to store passwords for me (or, I'd argue, for anyone).

Comment 50

Last year
To clarify Barbara and Andreas, there needs to be accessibility work done here to properly fix this, which will require help from the a11y team.
As Petru mentioned above this is actually two issues. The first is URL detection. I'm assuming some UI change caused a regression in how LastPass detect the current tab's URL.

The second issue is the accessible tree traversal to get and control the input fields. Even when we support a full Android a11y tree, it won't be a great experience. For one, walking the tree will block the UI and can potentially have perceivable performance hits. Outside of automated testing, the a11y tree should not be arbitrarily walked. For it to be performant we would need to have a full cache of the accessible tree available to the UI thread/process. Chrome does this, but we have decided to take a different approach.

A proper fix would be to implement the autofill framework. LastPass already supports it, it is a superior user experience, it is much more secure, and it won't cause a serious performance hit.
Flags: needinfo?(eitan)

Comment 52

Last year
(In reply to Eitan Isaacson [:eeejay] from comment #51)
> A proper fix would be to implement the autofill framework. LastPass already
> supports it, it is a superior user experience, it is much more secure, and
> it won't cause a serious performance hit.

I support this proper fix, though Autofill only works on Android O (https://developer.android.com/guide/topics/text/autofill), which is only ~6% of the Android user base currently: https://developer.android.com/about/dashboards/

I'll still wait for Andreas and/or Barbara to figure out priorities.
Thanks Eitan!

From my investigations and from what Kyle confirmed (thanks again!) for Oreo's autofill framework to work we would also need to expose the ViewNode tree which would contain HtmlInfo properties which carry information about the input fields on the page.
This is needed for the framework to actually know what fields need to be autofilled.

What WebView has done to support Oreo's autofill:
https://github.com/aosp-mirror/platform_frameworks_base/blob/master/core/java/android/webkit/WebView.java#L2710

For how Bitwarden supports the autofill:
https://github.com/bitwarden/mobile/blob/master/src/Android/Autofill/Parser.cs#L88
https://github.com/bitwarden/mobile/blob/master/src/Android/Autofill/Field.cs#L31
https://github.com/bitwarden/mobile/blob/master/src/Android/Autofill/FieldCollection.cs#L301

I saw Dashlane also stated they cannot support autofill for Firefox because "as of today it is not possible for us to access the web page on this browser" - https://blog.dashlane.com/android-p-autofill/

An important note also is that besides username and password there a lot of other fields that could be autofilled with data stored in 3rd party password managers.

NI'ing Anatoly to confirm if LastPass would also need the html fields exposed in order for autofill to work.
Flags: needinfo?(anatoly)
Thanks for the investigations, everyone. Is supporting autofill also required for Lockbox (and other password managers) to work with Fennec?
Flags: needinfo?(abovens)

Comment 55

Last year
I believe so Andreas, we implemented the Autofill API in Focus to support password managers.

Comment 56

Last year
(In reply to Petru-Mugurel Lingurar[:petru] from comment #53)

> NI'ing Anatoly to confirm if LastPass would also need the html fields
> exposed in order for autofill to work.

If you can provide hints for the input fields, per https://developer.android.com/guide/topics/text/autofill#hints, then we shouldn't need much else. However, if there are no hints, and we have to use heuristics to identify fields, then we will need labels at the very least, but the more information you can provide about the page structure, the easier it will be to identify the correct fields to fill.
Flags: needinfo?(anatoly)
Andreas and I confirm this is not a P1 for now.

Comment 58

Last year
I'm taking this off the Softvision Dev Crew's list as it looks to be a Gecko issue and Sebastian has asked to have it fixed in GV.
Whiteboard: [Leanplum] [61]
(In reply to Barbara Bermes [:barbara] from comment #57)
> Andreas and I confirm this is not a P1 for now.

(In reply to :sdaswani only needinfo from comment #58)
> I'm taking this off the Softvision Dev Crew's list as it looks to be a Gecko
> issue and Sebastian has asked to have it fixed in GV.

Susheel, what is the priority of this bug for GeckoView? This Gecko bug will be a functional regression when we migrate Focus from WebView to GeckoView.

In comment 57, Barbara said this bug is not a P1 for now, but the Bugzilla Priority field was never changed and is still P1.

In comment 58, you said Sebastian asked for the GeckoView team to fix this bug, but it's not on our radar atm.
Flags: needinfo?(sdaswani)
Summary: LastPass auto-fill has broken in latest version of Firefox for Android → LastPass and 1Password auto-fill is broken in Firefox for Android
Whiteboard: [geckoview:klar]
(In reply to Chris Peterson [:cpeterson] from comment #59)
> (In reply to :sdaswani only needinfo from comment #58)
> > I'm taking this off the Softvision Dev Crew's list as it looks to be a Gecko
> > issue and Sebastian has asked to have it fixed in GV.
> 
> 
> In comment 58, you said Sebastian asked for the GeckoView team to fix this
> bug, but it's not on our radar atm.

For supporting the Autofill Framework - bug 1352011, solution suggested by Eitan in comment #51 for fixing this bug, Sebastian created and linked [1] bug 1461961 which I see still needs investigation - bug 1461961 comment #6

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1352011#a35610415_472516

Comment 61

Last year
Chris the priority is up to Barbara and Andreas since it will affect Fenix.
Flags: needinfo?(sdaswani) → needinfo?(abovens)
Reporter

Comment 62

11 months ago
May I suggest that for a browser whose selling points include a focus on privacy, security and user agency, not supporting password managers ought to be high priority? It's been 2 years now, I doubt I'm the only user that's stopped using Firefox for Android because of this and I've certainly stopped recommending it to people too.

The built-in password management is woefully inadequate compared to the services that dedicated, 3rd-party managers provide, and given how many sites force you to use their apps, a password manager that only works for the browser is just not good enough anymore.
Summary: LastPass and 1Password auto-fill is broken in Firefox for Android → LastPass, 1Password, Bitwarden and all 3rd-party password manager auto-fill is broken in Firefox for Android
We're investigating the best path forward here.
Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195

Needinfo :susheel if you think this bug should be re-triaged.
Priority: P1 → P5
Jim can work on this in spare cycles. I think product will want this soon enough.
Assignee: nobody → nchen
Priority: P5 → P2
Priority: P2 → P1
P1 bug that we may want to fix for GV 62 Beta.

Jeff Boek confirmed that password managers work in Focus+WebView on Android 7.0 (so not using Android 8.0's new Autofill API) and not in Focus+GV.
[geckoview:klar:p1] because Jim is actively working on this bug.
Component: General → Logins, Passwords and Form Fill
Whiteboard: [geckoview:klar] → [geckoview:klar:p1]
Move the AccessibilityNodeProvider implementation under
SessionAccessibility, to reduce the indent of the code.

Also make all methods in SessionAccessibility.Settings static to make
the code easier to follow.
Make a distinction between a session being active (i.e. being visible)
and it being focused. More than one session may be active at a time, but
only one session is focused at a time. This means the focused session is
always active, but an active session may not be focused.

Also, manage setting of active/focused states in GeckoView itself, so
consumers don't generally have to worry about these states.
Make the session store event listeners inline, because it makes the code
more readable, and also because auto-fill requires a pageshow listener
that is always registered, so the existing pageshow listener needs to be
moved elsewhere.
Add an auto-fill backend in GeckoViewContent.js that detects fields on
the page and sends information about the fields to Java through events.
Add an auto-fill frontend that listens to events from Gecko. It
populates accessibility nodes for input fields and sends accessibility
events, in order to support auto-fill clients that use accessibility
services to perform auto-fill.
Add some tests to AccessibilityTest to make sure we can perform
auto-fill through the accessibility API.
Attachment #8999733 - Flags: review?(eitan)
Attachment #8999737 - Flags: review?(eitan)
Attachment #8999738 - Flags: review?(eitan)
Comment on attachment 8999733 [details]
Bug 1330257 - 1. Refactor SessionAccessibility; r=eeejay

Eitan Isaacson [:eeejay] has approved the revision.
Attachment #8999733 - Flags: review+
Comment on attachment 8999734 [details]
Bug 1330257 - 2. Separate out session focused state from active state; r=droeh

Dylan Roeh (:droeh) has approved the revision.
Attachment #8999734 - Flags: review+
Comment on attachment 8999735 [details]
Bug 1330257 - 3. Inline session store event listeners; r=droeh

Dylan Roeh (:droeh) has approved the revision.
Attachment #8999735 - Flags: review+
Comment on attachment 8999737 [details]
Bug 1330257 - 5. Add auto-fill accessibility frontend; r=eeejay

Eitan Isaacson [:eeejay] has approved the revision.
Attachment #8999737 - Flags: review+
Comments on phabricator don't end up here, so I am posting this from my last r+:

> I'm not that thrilled about this change.
> 
> It adds another tree into the mix that is implemented outside of a11y. The implemented cache is pretty naive and
> doesn't take any dom mutations into account. It doesn't actually calculate visibility of the input fields, and it
> dispatches accessible events from multiple sources (android/dom).
> 
> On the other hand, if this is a priority for 62 I can't think of any alternatives. I hope to have a version of this
> that full relies on a11y at some point after that.
Comment on attachment 8999738 [details]
Bug 1330257 - 6. Add tests for auto-fill accessibility frontend; r=eeejay

Eitan Isaacson [:eeejay] has approved the revision.
Attachment #8999738 - Flags: review+
Comment on attachment 8999736 [details]
Bug 1330257 - 4. Add auto-fill backend; r=esawin

Eugen Sawin [:esawin] on leave from 20 August has approved the revision.
Attachment #8999736 - Flags: review+
Comment on attachment 8999738 [details]
Bug 1330257 - 6. Add tests for auto-fill accessibility frontend; r=eeejay

Eitan Isaacson [:eeejay] has requested changes to the revision.
Attachment #8999738 - Flags: review+

Updated

10 months ago
Attachment #8999733 - Attachment description: Bug 1330257 - 1. Refactor SessionAccessibility; r?eeejay → Bug 1330257 - 1. Refactor SessionAccessibility; r=eeejay

Updated

10 months ago
Attachment #8999734 - Attachment description: Bug 1330257 - 2. Separate out session focused state from active state; r?droeh → Bug 1330257 - 2. Separate out session focused state from active state; r=droeh

Updated

10 months ago
Attachment #8999735 - Attachment description: Bug 1330257 - 3. Inline session store event listeners; r?droeh → Bug 1330257 - 3. Inline session store event listeners; r=droeh

Updated

10 months ago
Attachment #8999736 - Attachment description: Bug 1330257 - 4. Add auto-fill backend; r?esawin → Bug 1330257 - 4. Add auto-fill backend; r=esawin

Updated

10 months ago
Attachment #8999737 - Attachment description: Bug 1330257 - 5. Add auto-fill accessibility frontend; r?eeejay → Bug 1330257 - 5. Add auto-fill accessibility frontend; r=eeejay

Updated

10 months ago
Attachment #8999738 - Attachment description: Bug 1330257 - 6. Add tests for auto-fill accessibility frontend; r?eeejay → Bug 1330257 - 6. Add tests for auto-fill accessibility frontend; r=eeejay
Add a frontend for the Oreo auto-fill API in SessionTextInput, which
processes events from Gecko and provides consumer APIs that match the
Oreo auto-fill APIs. GeckoView then forwards the necessary calls to
SessionTextInput.
Comment on attachment 9001720 [details]
Bug 1330257 - 7. Add Oreo auto-fill frontend; r=esawin r=snorp

Eugen Sawin [:esawin] on leave from 20 August has approved the revision.
Attachment #9001720 - Flags: review+
Comment on attachment 9001720 [details]
Bug 1330257 - 7. Add Oreo auto-fill frontend; r=esawin r=snorp

James Willcox (:snorp) (jwillcox@mozilla.com) has approved the revision.
Attachment #9001720 - Flags: review+

Updated

10 months ago
Attachment #9001720 - Attachment description: Bug 1330257 - 7. Add Oreo auto-fill frontend; r?esawin r?snorp → Bug 1330257 - 7. Add Oreo auto-fill frontend; r=esawin r=snorp
Add some tests for the Oreo auto-fill frontend, similar to the tests for
the a11y auto-fill frontend. However, because these tests depend on the
ViewStructure class, they require SDK 23+ to run.

Comment 86

10 months ago
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7186e64192f7
1. Refactor SessionAccessibility; r=eeejay
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa0ef9b0e96a
2. Separate out session focused state from active state; r=droeh
https://hg.mozilla.org/integration/mozilla-inbound/rev/9085b9fd557c
3. Inline session store event listeners; r=droeh
https://hg.mozilla.org/integration/mozilla-inbound/rev/593f9a799d11
4. Add auto-fill backend; r=esawin
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c589fc8621b
5. Add auto-fill accessibility frontend; r=eeejay
https://hg.mozilla.org/integration/mozilla-inbound/rev/62e53fa35d5b
6. Add tests for auto-fill accessibility frontend; r=eeejay
https://hg.mozilla.org/integration/mozilla-inbound/rev/53ef69afca69
7. Add Oreo auto-fill frontend; r=esawin r=snorp
Keywords: leave-open
Backed out 8 changesets (bug 1330257) for Geckoview failures at GeckoSessionTestRuleTest.waitForPageStop_throwOnChangedCallback on a CLOSED TREE

Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/3aca6b42ca4efa4d5f1f08d8191e0699d4e0b86a

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=53ef69afca6959ca87e5c4cfb97955371d729fab

Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=195002592&repo=mozilla-inbound&lineNumber=1562

Log snippet: 
[task 2018-08-21T03:10:06.718Z] 03:10:06     INFO -  TEST-START | org.mozilla.geckoview.test.FinderTest.find_linksOnly
[task 2018-08-21T03:12:15.568Z] 03:12:15     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: id=AndroidJUnitRunner
[task 2018-08-21T03:12:15.569Z] 03:12:15     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: current=13
[task 2018-08-21T03:12:15.570Z] 03:12:15     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: class=org.mozilla.geckoview.test.FinderTest
[task 2018-08-21T03:12:15.570Z] 03:12:15     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: stream=
[task 2018-08-21T03:12:15.570Z] 03:12:15     INFO -  org.mozilla.geckoview.test | Error in find_linksOnly(org.mozilla.geckoview.test.FinderTest):
[task 2018-08-21T03:12:15.570Z] 03:12:15     INFO -  org.mozilla.geckoview.test | org.mozilla.geckoview.test.rdp.RDPConnection$TimeoutException: Socket timeout
[task 2018-08-21T03:12:15.570Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.rdp.RDPConnection$InputThread.processPacket(RDPConnection.java:113)
[task 2018-08-21T03:12:15.571Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.rdp.RDPConnection.processInputPacket(RDPConnection.java:314)
[task 2018-08-21T03:12:15.572Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.rdp.Actor$Reply.get(Actor.java:72)
[task 2018-08-21T03:12:15.573Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.rdp.Promises.attach(Promises.java:45)
[task 2018-08-21T03:12:15.578Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.rdp.Promises.<init>(Promises.java:38)
[task 2018-08-21T03:12:15.581Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.rdp.Tab.getPromises(Tab.java:77)
[task 2018-08-21T03:12:15.583Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.cleanupStatement(GeckoSessionTestRule.java:1446)
[task 2018-08-21T03:12:15.584Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.rule.GeckoSessionTestRule$4.evaluate(GeckoSessionTestRule.java:1489)
[task 2018-08-21T03:12:15.585Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at android.support.test.internal.statement.UiThreadStatement$1.run(UiThreadStatement.java:44)
[task 2018-08-21T03:12:15.585Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at android.app.Instrumentation$SyncRunnable.run(Instrumentation.java:1719)
[task 2018-08-21T03:12:15.587Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at android.os.Handler.handleCallback(Handler.java:730)
[task 2018-08-21T03:12:15.587Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at android.os.Handler.dispatchMessage(Handler.java:92)
[task 2018-08-21T03:12:15.589Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at android.os.Looper.loop(Looper.java:137)
[task 2018-08-21T03:12:15.589Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at android.app.ActivityThread.main(ActivityThread.java:5103)
[task 2018-08-21T03:12:15.589Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at java.lang.reflect.Method.invokeNative(Native Method)
[task 2018-08-21T03:12:15.590Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at java.lang.reflect.Method.invoke(Method.java:525)
[task 2018-08-21T03:12:15.590Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:737)
[task 2018-08-21T03:12:15.590Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:553)
[task 2018-08-21T03:12:15.590Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at dalvik.system.NativeStart.main(Native Method)
[task 2018-08-21T03:12:15.590Z] 03:12:15     INFO -  org.mozilla.geckoview.test |
[task 2018-08-21T03:12:15.591Z] 03:12:15     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: numtests=88
[task 2018-08-21T03:12:15.591Z] 03:12:15     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: stack=org.mozilla.geckoview.test.rule.GeckoSessionTestRule$ChildCrashedException: Child process crashed
[task 2018-08-21T03:12:15.591Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.rule.GeckoSessionTestRule$1.invoke(GeckoSessionTestRule.java:1178)
[task 2018-08-21T03:12:15.591Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at $Proxy21.onCrash(Native Method)
[task 2018-08-21T03:12:15.591Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.GeckoSession$1.handleMessage(GeckoSession.java:135)
[task 2018-08-21T03:12:15.591Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.GeckoSession$1.handleMessage(GeckoSession.java:126)
[task 2018-08-21T03:12:15.592Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.GeckoSessionHandler.handleMessage(GeckoSessionHandler.java:79)
[task 2018-08-21T03:12:15.592Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.gecko.EventDispatcher$2.run(EventDispatcher.java:344)
[task 2018-08-21T03:12:15.592Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at android.os.Handler.handleCallback(Handler.java:730)
[task 2018-08-21T03:12:15.593Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at android.os.Handler.dispatchMessage(Handler.java:92)
[task 2018-08-21T03:12:15.593Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at android.os.Looper.loop(Looper.java:137)
[task 2018-08-21T03:12:15.593Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at android.app.ActivityThread.main(ActivityThread.java:5103)
[task 2018-08-21T03:12:15.594Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at java.lang.reflect.Method.invokeNative(Native Method)
[task 2018-08-21T03:12:15.594Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at java.lang.reflect.Method.invoke(Method.java:525)
[task 2018-08-21T03:12:15.595Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:737)
[task 2018-08-21T03:12:15.595Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:553)
[task 2018-08-21T03:12:15.597Z] 03:12:15     INFO -  org.mozilla.geckoview.test | 	at dalvik.system.NativeStart.main(Native Method)
[task 2018-08-21T03:12:15.599Z] 03:12:15     INFO -  org.mozilla.geckoview.test |
[task 2018-08-21T03:12:15.599Z] 03:12:15     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: test=find_linksOnly
[task 2018-08-21T03:12:15.600Z] 03:12:15     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS_CODE: -2
[task 2018-08-21T03:12:15.600Z] 03:12:15  WARNING -  TEST-UNEXPECTED-FAIL | org.mozilla.geckoview.test.FinderTest.find_linksOnly | status -2
Flags: needinfo?(nchen)
Blocks: 1485107

Comment 89

10 months ago
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c8789a613477
1. Refactor SessionAccessibility; r=eeejay
https://hg.mozilla.org/integration/mozilla-inbound/rev/20a3dacdf9b8
2. Separate out session focused state from active state; r=droeh
https://hg.mozilla.org/integration/mozilla-inbound/rev/05566ea3c85e
3. Inline session store event listeners; r=droeh
https://hg.mozilla.org/integration/mozilla-inbound/rev/5f44c144b1e5
4. Add auto-fill backend; r=esawin
https://hg.mozilla.org/integration/mozilla-inbound/rev/6541f81c8e25
5. Add auto-fill accessibility frontend; r=eeejay
https://hg.mozilla.org/integration/mozilla-inbound/rev/d91afaee03b0
6. Add tests for auto-fill accessibility frontend; r=eeejay
https://hg.mozilla.org/integration/mozilla-inbound/rev/d0e40284db4b
7. Add Oreo auto-fill frontend; r=esawin r=snorp
Flags: needinfo?(nchen)
Comment on attachment 8999733 [details]
Bug 1330257 - 1. Refactor SessionAccessibility; r=eeejay

This would be nice to have on 62 for Focus, but it might be too late and the risk might be too big.

Approval Request Comment
[Feature/Bug causing the regression]: N/A
[User impact if declined]: Focus using 62 won't have auto-fill feature
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: No
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Somewhat
[Why is the change risky/not risky?]: Lots of new code, though only custom tabs / web apps are affected for Fennec; i.e. the new code is not active for the main Fennec app.
[String changes made/needed]: None
Attachment #8999733 - Flags: approval-mozilla-beta?
Comment on attachment 9002553 [details]
Bug 1330257 - 8. Add tests for Oreo auto-fill frontend; r?snorp

James Willcox (:snorp) (jwillcox@mozilla.com) has approved the revision.
Attachment #9002553 - Flags: review+
This seems really scary to land so late in the cycle. What's the impact to Focus if we let this ride the 63 trains instead?
Flags: needinfo?(nchen)
I think it really depends on the Focus product plan for GeckoView at this point, which I'm not entirely sure about. It gets a little complicated since this bug might affect that product plan as well. This bug is risky enough though that I think it makes sense to let it ride the trains at this point.
Flags: needinfo?(nchen)
Comment on attachment 8999733 [details]
Bug 1330257 - 1. Refactor SessionAccessibility; r=eeejay

Unfortunately, I think we're too late in the cycle to take such a large change, even if the risk to Fennec is somewhat-contained to custom tabs and web apps. There just isn't enough time to find regressions before Fennec 62 is due to ship.

Per discussion with cpeterson, I think there are alternative options for shipping these patches in a GV62-based Focus release and I believe he's planning to start an email thread to discuss.
Attachment #8999733 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
BTW, I hit conflicts already on Part 1 when seeing how well this would graft to Beta. So some rebasing will also be necessary.

Comment 97

10 months ago
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3758580bd889
8. Add tests for Oreo auto-fill frontend; r=snorp
Blocks: 1485810
I changed this bug to track auto-fill work in GeckoView. Currently the main Fennec app still doesn't support auto-fill because it's not entirely based on GeckoView. Bug 1485810 will track additional work to make auto-fill function in Fennec.
Component: Logins, Passwords and Form Fill → GeckoView
Keywords: leave-open
Summary: LastPass, 1Password, Bitwarden and all 3rd-party password manager auto-fill is broken in Firefox for Android → LastPass, 1Password, Bitwarden and all 3rd-party password manager auto-fill is broken in GeckoView

Comment 99

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3758580bd889
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Blocks: 1486571
Duplicate of this bug: 1461961
Depends on: 1489250
Reporter

Updated

10 months ago
Depends on: 1489345
Depends on: 1491989
Blocks: 1491989
No longer depends on: 1491989
Flags: needinfo?(abovens)
Depends on: 1495822
Depends on: 1504257

Updated

6 months ago
Product: Firefox for Android → GeckoView
Keywords: regression
Target Milestone: Firefox 63 → mozilla63
Flags: needinfo?(bbermes)
You need to log in before you can comment on or make changes to this bug.