1627651 - Grey key icon retains highlight after the MP is closed

Status: RESOLVED WORKSFORME

(Toolkit :: Password Manager, defect, P2)

Description

[Timea Cernea [:tbabos][inactive]]

Attachment: Recording of the issue

Affected Versions
Nightly 76.0a1 (2020-04-05)

Tested on
Windows 10 x64

Steps to reproduce

1. Have a profile with MP and saved credentials on Facebook
2. Go to Facebook, dismiss the MP prompt when it appears
3. Fill in the username/password fields and click out
4. Dismiss the MP prompt once again and click on the grey key icon
5. FIll in the correct password for MP and don't move the mouse

Expected
The key grey icon should not retain the highlight after the MP is dismissed

Actual
As long as there are no mouse movements, the highlight will remain displayed.
Browser error:

can't access property "mainAction", currentNotification is null LoginManagerPrompter.jsm:221
updateButtonLabel resource://gre/modules/LoginManagerPrompter.jsm:221
writeDataToUI resource://gre/modules/LoginManagerPrompter.jsm:252
eventCallback resource://gre/modules/LoginManagerPrompter.jsm:524
PopupNotifications_fireCallback resource://gre/modules/PopupNotifications.jsm:1672
_popupshownListener resource://gre/modules/PopupNotifications.jsm:1308
forEach self-hosted:225
_popupshownListener resource://gre/modules/PopupNotifications.jsm:1307
<anonymous> self-hosted:844
(Async: EventListener.handleEvent)
PopupNotifications_showPanel resource://gre/modules/PopupNotifications.jsm:1319

Regression-Range
Introduced via the implementation of Dismissed Doorhanger, Bug 1536728.

Note
Beta 75 has the pref disabled for the dismiss doorhanger.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Comment 2

[Sam Foster [:sfoster] (he/him)]

The highlight here is a hover style applied to the icon: https://searchfox.org/mozilla-central/source/browser/themes/shared/identity-block/identity-block.inc.css#44

I'm not sure if the exception stack in comment #0 is causal or just associated with this symptom. The highlight isn't added or removed via code that this exception would directly interfere with, but it is suggestive of a problem that warrants further investigation.

I'm also not sure if this is actually a regression caused by Bug 1536728, or if that bug makes it more likely to have a dismissed doorhanger for this symptom to appear on. Setting P2 for now, but due to the specific steps needed to reproduce and minor user impact we may downgrade it further.

Comment 3

[Matthew N. [:MattN]]

IMO the bigger usability problem shown in the video is the MP prompt upon tabbing out of the password field… I guess we can't avoid that now with the current architecture if we want to show the key icon since we need to know whether to show a save or update doorhanger.

When we make the prompt contents able to be updated maybe we should consider a single API for both then we could delay all rendering and storage queries until the popup is opened (thus delaying the MP prompt until it's more relevant).

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

:tbabos, since this bug is a regression, could you fill (if possible) the regressed_by field?
For more information, please visit auto_nag documentation.

Comment 5

[Timea Cernea [:tbabos][inactive]]

Can no longer reproduce this on latest Nightly 82.0a1 (2020-08-24) (64-bit) due to the latest changes on how the dismissed doorhanger works when Primary Password is still locked, for more info please check https://bugzilla.mozilla.org/show_bug.cgi?id=1653945#c3.