1193343 - Detect dynamic removals of password fields from a FormLike

Status: RESOLVED INCOMPLETE

(Toolkit :: Password Manager, defect, P5)

Description

[:Paolo Amadini]

The FormLike objects used to gather information about password fields in pages are not live lists, so removals are not easily detected.

I suggest we spend some time in understanding how we could streamline the logic and caches used there so that we can easily detect removals as well. This will be useful for updating the current state of insecure password fields.

Comment 1

[Brian Grinstead [:bgrins]]

(In reply to :Paolo Amadini from comment #0)
> The FormLike objects used to gather information about password fields in
> pages are not live lists, so removals are not easily detected.

Sounds like removals are not detected in any case (not just subframes).  FormLike objects seem to be created from a form or individual field, so frames wouldn't be a part of them if I'm understanding correctly.  Is this bug specific to subframes as the title indicates, or is it just about detecting removals of password fields from a form?

Comment 2

[Brian Grinstead [:bgrins]]

Also, it sounds like this is maybe blocked by some ongoing work.  Can you mark it as dependent if so?  I'm  going to grab a different bug for now instead.

Comment 3

[Tanvi Vyas[:tanvi]]

Assume http://a.com has a password field.  Assume it frames http://b.com that also has a password field.  We mark the top level page with a security state STATE_HAS_INSECURE_PASSWORD.  The user interacts with the b.com frame and the password field is dynamically removed.  On dynamic removal, how will we know if we should remove STATE_HAS_INSECURE_PASSWORD from the securityUI of the top level page?  

I'm not entirely convinced this is required for the bugs it blocks.  We can mark pages that once had a password field as insecure and then later detect removals and attempt to change the security UI in those cases.  Persisting the negative HTTP icon is not the worst issue, since eventually we want to deprecate HTTP anyway.

Comment 4

[:Paolo Amadini]

(In reply to Brian Grinstead [:bgrins] from comment #1)
> Is this
> bug specific to subframes as the title indicates, or is it just about
> detecting removals of password fields from a form?

It's not specific to subframes, sorry if the title was misleading. I specified that since it's a case we want to think about explicitly. The original idea behind this bug was to use the logic already implemented for the Login Fill Doorhanger, but after discussing with Tanvi we may want to work inside the C++ objects in the platform to detect insecure passwords so it's not clear what approach we should take here.

(In reply to Brian Grinstead [:bgrins] from comment #2)
> Also, it sounds like this is maybe blocked by some ongoing work.  Can you
> mark it as dependent if so?  I'm  going to grab a different bug for now
> instead.

It's not blocked as much by other work as by figuring out a more precise plan (I'm working on this with Tanvi and Matt). I don't know how to express this in Bugzilla so I put an "[awaiting architecture]" tag, feel free to edit if you have a better suggestion.

(In reply to Tanvi Vyas [:tanvi] from comment #3)
> On dynamic removal, how will
> we know if we should remove STATE_HAS_INSECURE_PASSWORD from the securityUI
> of the top level page?  

We have to aggregate the data both on removal and on navigation, similarly to what the Login Fill Doorhanger logic does.

> I'm not entirely convinced this is required for the bugs it blocks.  We can
> mark pages that once had a password field as insecure and then later detect
> removals and attempt to change the security UI in those cases.  Persisting
> the negative HTTP icon is not the worst issue, since eventually we want to
> deprecate HTTP anyway.

That's a good point, but we should still track this bug because it may be quite cheap to do it once we've implemented the actual state handling.

Comment 5

[Matthew N. [:MattN]]

(In reply to :Paolo Amadini from comment #4)

(In reply to Tanvi Vyas [:tanvi] from comment #3)

On dynamic removal, how will
we know if we should remove STATE_HAS_INSECURE_PASSWORD from the securityUI
of the top level page?

We have to aggregate the data both on removal and on navigation, similarly
to what the Login Fill Doorhanger logic does.

I'm not entirely convinced this is required for the bugs it blocks. We can
mark pages that once had a password field as insecure and then later detect
removals and attempt to change the security UI in those cases. Persisting
the negative HTTP icon is not the worst issue, since eventually we want to
deprecate HTTP anyway.

That's a good point, but we should still track this bug because it may be
quite cheap to do it once we've implemented the actual state handling.

Marking as P5 because I agree this isn't that bad.

Comment 6

[Sergey Galich [:serg]]

Some web sites can remove inputs as part of their submit process, this would make us miss inputs.
It's unclear what problem we are trying to solve. Lets reopen when we have a problem definition.