1514255 - Skip passwordmgr tests under GeckoView

Status: RESOLVED FIXED

(GeckoView :: General, enhancement)

Description

[James Willcox (:snorp) ([e-mail]) (he/him)]

We don't support it in GeckoView.

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Bug 1514255 - Skip passwordmgr tests on GeckoView as it's unsupported r?MattN

Comment 2

[Matthew N. [:MattN]]

(In reply to James Willcox (:snorp) ([e-mail]) 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.

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from comment #2)
> (In reply to James Willcox (:snorp) ([e-mail]) 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.

Comment 4

[Matthew N. [:MattN]]

(In reply to James Willcox (:snorp) ([e-mail]) 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) ([e-mail]) 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.

Comment 5

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from comment #4)
> (In reply to James Willcox (:snorp) ([e-mail]) 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) ([e-mail]) 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.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fac4a156c22d
Skip passwordmgr tests on GeckoView as it's unsupported r=MattN

Comment 7

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/fac4a156c22d