Closed Bug 1514255 Opened 2 years ago Closed 2 years ago

Skip passwordmgr tests under GeckoView

Categories

(GeckoView :: General, enhancement)

enhancement
Not set
normal

Tracking

(firefox66 fixed)

RESOLVED FIXED
mozilla66
Tracking Status
firefox66 --- fixed

People

(Reporter: snorp, Assigned: snorp)

References

Details

Attachments

(1 file)

We don't support it in GeckoView.
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #0)
> We don't support it in GeckoView.

What does this mean? That no GeckoView apps will be able to use the Toolkit Password Manager? It seems like it would be required for Fenix.

There have been code changes in the Password Manager code to support Gecko View so I'm trying to understand what the actual level of support is.
Flags: needinfo?(snorp)
(In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from comment #2)
> (In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #0)
> > We don't support it in GeckoView.
> 
> What does this mean? That no GeckoView apps will be able to use the Toolkit
> Password Manager? It seems like it would be required for Fenix.
> 
> There have been code changes in the Password Manager code to support Gecko
> View so I'm trying to understand what the actual level of support is.

AFAIK I don't think we'll want Gecko to store passwords itself, so it won't make sense to use the Gecko password autofill. We'll likely make use of Android's autofill support instead, enabling us to take advantage of Lockbox and other services. Regardless, the password autofill support is not hooked up to GeckoView as it stands today.
Flags: needinfo?(snorp)
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #3)
> (In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from
> comment #2)
> > (In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #0)
> > > We don't support it in GeckoView.
> > 
> > What does this mean? That no GeckoView apps will be able to use the Toolkit
> > Password Manager? It seems like it would be required for Fenix.
> > 
> > There have been code changes in the Password Manager code to support Gecko
> > View so I'm trying to understand what the actual level of support is.
> 
> AFAIK I don't think we'll want Gecko to store passwords itself, so it won't
> make sense to use the Gecko password autofill.

Hmm… that's a change in direction… when we had the build/buy/partner decision that led to the creation of the Lockbox team, it was decided that password management should be built-into Firefox.

> We'll likely make use of
> Android's autofill support instead, enabling us to take advantage of Lockbox
> and other services. 

That is using password manager code IIUC, maybe not the autofill part but it seems like the part for finding the username/password fields is or will be used (e.g. bug 1497682)

> Regardless, the password autofill support is not hooked
> up to GeckoView as it stands today.

Note that the tests you're disabling aren't only for the autofill portion, they're also for the heuristics deciding what is saved.
(In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from comment #4)
> (In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #3)
> > (In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from
> > comment #2)
> > > (In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #0)
> > > > We don't support it in GeckoView.
> > > 
> > > What does this mean? That no GeckoView apps will be able to use the Toolkit
> > > Password Manager? It seems like it would be required for Fenix.
> > > 
> > > There have been code changes in the Password Manager code to support Gecko
> > > View so I'm trying to understand what the actual level of support is.
> > 
> > AFAIK I don't think we'll want Gecko to store passwords itself, so it won't
> > make sense to use the Gecko password autofill.
> 
> Hmm… that's a change in direction… when we had the build/buy/partner
> decision that led to the creation of the Lockbox team, it was decided that
> password management should be built-into Firefox.
> 
> > We'll likely make use of
> > Android's autofill support instead, enabling us to take advantage of Lockbox
> > and other services. 
> 
> That is using password manager code IIUC, maybe not the autofill part but it
> seems like the part for finding the username/password fields is or will be
> used (e.g. bug 1497682)

Ah, yes, we may be using common code to find these fields. At least we should. I haven't really looked at that stuff deeply.

> 
> > Regardless, the password autofill support is not hooked
> > up to GeckoView as it stands today.
> 
> Note that the tests you're disabling aren't only for the autofill portion,
> they're also for the heuristics deciding what is saved.

OK, we may want to hook some things back up for that. But AFAIK we don't want toolkit's prompting or storage stuff for logins.
Pushed by jwillcox@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fac4a156c22d
Skip passwordmgr tests on GeckoView as it's unsupported r=MattN
https://hg.mozilla.org/mozilla-central/rev/fac4a156c22d
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66
Product: Firefox for Android → GeckoView
Target Milestone: Firefox 66 → mozilla66
You need to log in before you can comment on or make changes to this bug.