Chris Lord [:cwiiis] Reporter
	Description • 8 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.

	Barbara Bermes [:barbara] - NI please!
	Comment 1 • 8 years ago

LastPass is #9 of Top10 Android Add-ons, so we should get this verified and fixed soon.

NI Sebastian/Joe

	Sebastian Kaspari (:sebastian; :pocmo)
	Comment 2 • 8 years ago

Yeah, let's talk about this at triage tomorrow morning. We just missed the Firefox 51 release unfortunately.

	Wesley Huang [:wesley_huang] (EPM) (NI me)
	Comment 3 • 8 years ago

Hi Sorina,
could you provide a regression window?

	Jing-wei Wu [:jwu]
	Comment 4 • 8 years ago

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)

	Jing-wei Wu [:jwu]
	Comment 5 • 8 years ago

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.

	Chris Lord [:cwiiis] Reporter
	Comment 6 • 8 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...)

	Jing-wei Wu [:jwu]
	Comment 7 • 8 years ago

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.

	Chris Lord [:cwiiis] Reporter
	Comment 8 • 8 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.

	Joe Cheng [:jcheng] (please needinfo)
	Comment 9 • 8 years ago

clear the NI on me as the bug already has P1 Priority with tracking-fennec+

	Kevin Brosnan [Ex-Mozilla]
	Comment 10 • 8 years ago

Mike do we have any business contacts with LastPass? AMO team or mconnor might know more.

	Mike Kaply [:mkaply]
	Comment 11 • 8 years ago

I'm pretty sure we do. Copying kev who would know.

	Tim Guan-tin Chien [:timdream] (please needinfo)
	Comment 12 • 8 years ago

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).

	Kev Needham [:kev]
	Comment 13 • 8 years ago

Bill, I don't know if you're still talking with the LP team, but if you are, could you bring this up?

	Chris Lord [:cwiiis] Reporter
	Comment 14 • 8 years ago

Adding n?billm to answer comment #13. LastPass is still broken on Android, fwiw.

	Bill McCloskey [inactive unless it's an emergency] (:billm)
	Comment 15 • 8 years ago

Andrew might be able to help.

	Andrew Zitnay
	Comment 16 • 8 years ago

anatoly@lastpass.com would probably be the best person to triage this.

	Bill McCloskey [inactive unless it's an emergency] (:billm)
	Comment 17 • 8 years ago

Thanks Andrew. Can you help with this issue Anatoly?

	Anatoly
	Comment 18 • 8 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.

	Bill McCloskey [inactive unless it's an emergency] (:billm)
	Comment 19 • 8 years ago

I'm not sure who to needinfo about this. Snorp, can you redirect as needed based on comment 18?

	James Willcox (:snorp) (jwillcox@mozilla.com) (he/him)
	Comment 20 • 8 years ago

urlbar issue, so I think the frontend folks should be able to help here. Tim, can you respond to comment #18?

-> timdream

	Tim Guan-tin Chien [:timdream] (please needinfo)
	Comment 21 • 8 years ago

Max, do you have enough information here to work on this? Do we need a test to make sure accessibility won't regress?

	Max Liu [:maliu]
	Comment 22 • 8 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?

	Joe Cheng [:jcheng] (please needinfo)
	Comment 23 • 8 years ago

(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?

	Tim Guan-tin Chien [:timdream] (please needinfo)
	Comment 24 • 8 years ago

Yeah it looks like as long as our UI can be accessed by accessibility API we should be good, no?

	Anatoly
	Comment 25 • 8 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)

	Joe Cheng [:jcheng] (please needinfo)
	Comment 26 • 8 years ago

yup, looking at the Android O auto-fill feature as well

	Chris Lord [:cwiiis] Reporter
	Comment 27 • 7 years ago

Been another month, sorry for the spam but wanted to check if there's any progress on this?

	Tim Guan-tin Chien [:timdream] (please needinfo)
	Comment 28 • 7 years ago

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.

	Chris Lord [:cwiiis] Reporter
	Comment 29 • 7 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?

	Tim Guan-tin Chien [:timdream] (please needinfo)
	Comment 30 • 7 years ago

Unfortunately, I am no longer being assiged to Fennec. It's unclear if we will get to this.

	Nick Alexander :nalexander [he/him] (PTO until Montreallhands August 21, 2023)
	Comment 31 • 7 years ago

(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

	Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]
	Comment 32 • 7 years ago

[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.

	Eitan Isaacson [:eeejay]
	Comment 33 • 7 years ago

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..

	:sdaswani
	Comment 35 • 7 years ago

Mike I would keep this as a typical P1, not a critical one.

	Chris Lord [:cwiiis] Reporter
	Comment 36 • 7 years ago

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).

	Petru-Mugurel Lingurar [:petru]
	Comment 40 • 6 years ago

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

	Sorina Florean [:sflorean]
	Comment 41 • 6 years ago

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.

	Kyle
	Comment 42 • 6 years ago

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.

	Petru-Mugurel Lingurar [:petru]
	Comment 44 • 6 years ago

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.

	Petru-Mugurel Lingurar [:petru]
	Comment 45 • 6 years ago

NI'ing James to see if he thinks the suggested approach to expose web contents structure is doable.

	Kyle
	Comment 46 • 6 years ago

Yes, I can confirm that we have no issues autofilling in Focus. Accessibility and Oreo both work there using Bitwarden.

	James Willcox (:snorp) (jwillcox@mozilla.com) (he/him)
	Comment 47 • 6 years ago

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.

	:sdaswani
	Comment 48 • 6 years ago

Andreas and Barbara, can you prioritize this work for us? Is this something that must be fixed? And if so, by when?

	Chris Lord [:cwiiis] Reporter
	Comment 49 • 6 years ago

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).

	:sdaswani
	Comment 50 • 6 years ago

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.

	Eitan Isaacson [:eeejay]
	Comment 51 • 6 years ago

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.

	:sdaswani
	Comment 52 • 6 years ago

(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.

	Petru-Mugurel Lingurar [:petru]
	Comment 53 • 6 years ago

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.

	Andreas Bovens [:abovens]
	Comment 54 • 6 years ago

Thanks for the investigations, everyone. Is supporting autofill also required for Lockbox (and other password managers) to work with Fennec?

	:sdaswani
	Comment 55 • 6 years ago

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

	Anatoly
	Comment 56 • 6 years ago

(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.

	Barbara Bermes [:barbara]
	Comment 57 • 6 years ago

Andreas and I confirm this is not a P1 for now.

	:sdaswani
	Comment 58 • 6 years ago

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.

	Chris Peterson [:cpeterson]
	Comment 59 • 6 years ago

(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.

	Petru-Mugurel Lingurar [:petru]
	Comment 60 • 6 years ago

(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

	:sdaswani
	Comment 61 • 6 years ago

Chris the priority is up to Barbara and Andreas since it will affect Fenix.

	Chris Lord [:cwiiis] Reporter
	Comment 62 • 6 years 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.

	Andreas Bovens [:abovens]
	Comment 63 • 6 years ago

We're investigating the best path forward here.

	Firefox Bug Husbandry Bot
	Comment 64 • 6 years ago

Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195

Needinfo :susheel if you think this bug should be re-triaged.

	David Bolter [:davidb] (NeedInfo me for attention)
	Comment 65 • 6 years ago

Jim can work on this in spare cycles. I think product will want this soon enough.

	Chris Peterson [:cpeterson]
	Comment 66 • 6 years ago

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.

	Chris Peterson [:cpeterson]
	Comment 67 • 6 years ago

[geckoview:klar:p1] because Jim is actively working on this bug.

	(inactive) Jim Chen [:jchen] [:darchons] Assignee
	Comment 68 • 6 years ago

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.

	(inactive) Jim Chen [:jchen] [:darchons] Assignee
	Comment 69 • 6 years ago

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.

	(inactive) Jim Chen [:jchen] [:darchons] Assignee
	Comment 70 • 6 years ago

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.

	(inactive) Jim Chen [:jchen] [:darchons] Assignee
	Comment 71 • 6 years ago

Add an auto-fill backend in GeckoViewContent.js that detects fields on
the page and sends information about the fields to Java through events.

	(inactive) Jim Chen [:jchen] [:darchons] Assignee
	Comment 72 • 6 years ago

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.

	(inactive) Jim Chen [:jchen] [:darchons] Assignee
	Comment 73 • 6 years ago

Add some tests to AccessibilityTest to make sure we can perform
auto-fill through the accessibility API.

	Eitan Isaacson [:eeejay]
	Comment 74 • 6 years ago

Comment on attachment 8999733 [details]
Bug 1330257 - 1. Refactor SessionAccessibility; r=eeejay

Eitan Isaacson [:eeejay] has approved the revision.

	Dylan Roeh (:droeh) (he/him)
	Comment 75 • 6 years ago

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.

	Dylan Roeh (:droeh) (he/him)
	Comment 76 • 6 years ago

Comment on attachment 8999735 [details]
Bug 1330257 - 3. Inline session store event listeners; r=droeh

Dylan Roeh (:droeh) has approved the revision.

	Eitan Isaacson [:eeejay]
	Comment 77 • 6 years ago

Comment on attachment 8999737 [details]
Bug 1330257 - 5. Add auto-fill accessibility frontend; r=eeejay

Eitan Isaacson [:eeejay] has approved the revision.

	Eitan Isaacson [:eeejay]
	Comment 78 • 6 years ago

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.

	Eitan Isaacson [:eeejay]
	Comment 79 • 6 years ago

Comment on attachment 8999738 [details]
Bug 1330257 - 6. Add tests for auto-fill accessibility frontend; r=eeejay

Eitan Isaacson [:eeejay] has approved the revision.

	Eugen Sawin [:esawin]
	Comment 80 • 6 years ago

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.

	Eitan Isaacson [:eeejay]
	Comment 81 • 6 years ago

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.

	(inactive) Jim Chen [:jchen] [:darchons] Assignee
	Comment 82 • 6 years ago

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.

	Eugen Sawin [:esawin]
	Comment 83 • 6 years ago

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.

	James Willcox (:snorp) (jwillcox@mozilla.com) (he/him)
	Comment 84 • 6 years ago

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.

	(inactive) Jim Chen [:jchen] [:darchons] Assignee
	Comment 85 • 6 years ago

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.

	Pulsebot
	Comment 86 • 6 years 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

	Pulsebot
	Comment 87 • 6 years ago

Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c26970c767e1
5a. Fix checkstyle error; r=jchen

	Raul Gurzau (:RaulG)
	Comment 88 • 6 years ago

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

	Pulsebot
	Comment 89 • 6 years 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

	(inactive) Jim Chen [:jchen] [:darchons] Assignee
	Comment 90 • 6 years ago

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

	Tiberius Oros[:tiberius_oros]
	Comment 91 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/c8789a613477
https://hg.mozilla.org/mozilla-central/rev/20a3dacdf9b8
https://hg.mozilla.org/mozilla-central/rev/05566ea3c85e
https://hg.mozilla.org/mozilla-central/rev/5f44c144b1e5
https://hg.mozilla.org/mozilla-central/rev/6541f81c8e25
https://hg.mozilla.org/mozilla-central/rev/d91afaee03b0
https://hg.mozilla.org/mozilla-central/rev/d0e40284db4b

	James Willcox (:snorp) (jwillcox@mozilla.com) (he/him)
	Comment 92 • 6 years ago

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.

	Ryan VanderMeulen [:RyanVM]
	Comment 93 • 6 years ago

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?

	(inactive) Jim Chen [:jchen] [:darchons] Assignee
	Comment 94 • 6 years ago

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.

	Ryan VanderMeulen [:RyanVM]
	Comment 95 • 6 years ago

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.

	Ryan VanderMeulen [:RyanVM]
	Comment 96 • 6 years ago

BTW, I hit conflicts already on Part 1 when seeing how well this would graft to Beta. So some rebasing will also be necessary.

	Pulsebot
	Comment 97 • 6 years 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

	(inactive) Jim Chen [:jchen] [:darchons] Assignee
	Comment 98 • 6 years ago

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.

	Tiberius Oros[:tiberius_oros]
	Comment 99 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/3758580bd889