Closed Bug 1355534 Opened 3 years ago Closed 3 years ago

Revert to longer insecure login field contextual warning string

Categories

(Toolkit :: Password Manager, defect, P1)

53 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox52 --- unaffected
firefox53 ? fixed
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: MattN, Assigned: MattN)

References

Details

Attachments

(1 file)

A shorter string for the warning was landed in bug 1332901 to save on screen real estate at Ryan's request and we were wanting to do user research to compare the strings first but UR didn't have time to do it then. Based on research from the release of the feature in 52, it seems that users don't properly understand even the longer string so now we are doing the research.

UR already has results of comparing the 52 and 53 strings and has concluded that the original 52 string is more clear then the one currently on 53+ so we should revert to that already translated string. For 55 or 56 we may switch to a new string but that will be handled in a separate bug after further UR is completed.
Priority: -- → P1
Comment on attachment 8857048 [details]
Bug 1355534 - Revert to longer insecure login field contextual warning string.

Approval Request Comment
[Feature/Bug causing the regression]: Insecure login field warning feedback and user research
[User impact if declined]: Users may be more confused about the contextual insecure field warning
[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]: Probably not since this is just reverting to the old string now that we have user research
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Low
[Why is the change risky/not risky?]: Only risk would be about layout issues from a longer string but we're going back to the string that shipped in 52 so I don't think it will be any worse than that.
[String changes made/needed]: No, no string files are changing we're just using a different string.
Attachment #8857048 - Flags: approval-mozilla-beta?
Attachment #8857048 - Flags: approval-mozilla-aurora?
Comment on attachment 8857048 [details]
Bug 1355534 - Revert to longer insecure login field contextual warning string.

https://reviewboard.mozilla.org/r/128942/#review131486
Attachment #8857048 - Flags: review?(jhofmann) → review+
Comment on attachment 8857048 [details]
Bug 1355534 - Revert to longer insecure login field contextual warning string.

This would need to land on m-r for 53 release, noting it at the channel meeting so that I'll see it later today. 
Flod would this be a problem? I'm thinking it shouldn't be too bad since we are reverting to existing and already localized strings.
Flags: needinfo?(francesco.lodolo)
Attachment #8857048 - Flags: approval-mozilla-release?
It shouldn't be a problem. We're going to use a string that was already there before, used in the same context.

My only possible concern would be layout issues, in case something changed in the meantime. Matt would know that for sure.
Flags: needinfo?(francesco.lodolo)
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/autoland/rev/97f3a9003237
Revert to longer insecure login field contextual warning string. r=johannh
(In reply to Francesco Lodolo [:flod] from comment #5)
> My only possible concern would be layout issues, in case something changed
> in the meantime. Matt would know that for sure.

Off hand I don't think there were layout changes. Bug 1330731 is the only one in mind and that hasn't landed yet.
Comment on attachment 8857048 [details]
Bug 1355534 - Revert to longer insecure login field contextual warning string.

OK, this should land for the 53 RC2 build and the fennec RC build if this affects fennec too as I assume it may.
Attachment #8857048 - Flags: approval-mozilla-release?
Attachment #8857048 - Flags: approval-mozilla-release+
Attachment #8857048 - Flags: approval-mozilla-beta?
Attachment #8857048 - Flags: approval-mozilla-beta+
Attachment #8857048 - Flags: approval-mozilla-aurora?
Attachment #8857048 - Flags: approval-mozilla-aurora+
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #8)
> OK, this should land for the 53 RC2 build and the fennec RC build if this
> affects fennec too as I assume it may.

The code would affect Fennec but the feature is disabled there as it needs work by the FxAndroid team.

I'm not sure what the whole deal is with the beta tree closure so could a sheriff please do the beta+release uplifts? I'll post in #sheriffs I guess since I don't know who is sheriffing now.
Reminder for RyanVM to land on m-r.
Flags: needinfo?(ryanvm)
Flags: needinfo?(ryanvm)
https://hg.mozilla.org/mozilla-central/rev/97f3a9003237
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
(In reply to Matthew N. [:MattN] (behind on bugmail; PM if requests are blocking you) from comment #2)
> [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]: Probably not since
> this is just reverting to the old string now that we have user research

Setting qe-verify- based on Matt's assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.