896316 - Don't start controlling a read-only input even if it's marked as a login manager field

Status: RESOLVED FIXED

(Toolkit :: Form Manager, defect)

Description

[Matthew N. [:MattN]]

Attachment: v.1 Move the read-only check earlier

This way we can still mark a username field as one that login manager cares about without having to duplicate the read-only check in Password Manager. This also means we can keep a field marked as a login form despite @readonly so we don't need to watch for attribute changes in password manager.

Comment 1

[Matthew N. [:MattN]]

Try push: https://tbpl.mozilla.org/?tree=Try&rev=c795d3ab1896

Comment 2

[Justin Dolske [:Dolske]]

Comment on attachment 779037 [details] [diff] [review]
v.1 Move the read-only check earlier

Review of attachment 779037 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/satchel/nsFormFillController.cpp
@@ +863,5 @@
>  
>    nsCOMPtr<nsIFormControl> formControl = do_QueryInterface(input);
>    if (isPwmgrInput || (formControl &&
>                         formControl->IsSingleLineTextControl(true) &&
> +                       (hasList || autocomplete))) {

A bit of driveby cleanup, but I think you could move the |formControl| checks up to the top too, which would simplify the conditional here.

I don't see how we'd ever get isPwmgrInput == true without IsSingleLineTextControl() also being true.

Comment 3

[Matthew N. [:MattN]]

Attachment: Part 2 - Cleanup nsFormFillController::Focus to not treat password fields specially

(In reply to Justin Dolske [:Dolske] from comment #2)
> A bit of driveby cleanup, but I think you could move the |formControl|
> checks up to the top too, which would simplify the conditional here.
> 
> I don't see how we'd ever get isPwmgrInput == true without
> IsSingleLineTextControl() also being true.

Yeah, I thought the same but didn't want to mix it into the same patch in order to make reviewing easier. Here is a simple patch that cleans the rest up. Form Manager and Password Manager tests pass locally.

Comment 4

[Matthew N. [:MattN]]

https://hg.mozilla.org/integration/fx-team/rev/10d4c53564bc
https://hg.mozilla.org/integration/fx-team/rev/cb81405573fe

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/10d4c53564bc
https://hg.mozilla.org/mozilla-central/rev/cb81405573fe