Closed Bug 1638587 Opened 5 months ago Closed 3 months ago

Dismissed login capture doorhanger does not show when password field is edited and in its own Shadow Root

Categories

(Toolkit :: Password Manager, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla80
Tracking Status
firefox80 --- verified

People

(Reporter: bdanforth, Assigned: bdanforth)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file, 1 obsolete file)

Based on the _onPasswordEditedOrGenerated callback, a dismissed doorhanger should appear when:

  • The user edits a password field (Bug 1536728)
  • The user edits a username field (when a matching password field has already been filled)

(There may be other cases as well that I'm not aware of yet.)

Currently, for the two above cases the dismissed capture on edit doorhanger does not show when the username and password fields are each in their own shadow root (e.g. https://www.virustotal.com/gui/sign-in from Bug 1634819).

This bug is concerned with fixing the first case, when the password field is edited.

STR:

  1. Go to https://www.virustotal.com/gui/sign-in
  2. Enter some value(s) into the password field
  3. Blur the field

Actual results:
There is no key icon in the URL bar.
This would, when clicked, open the dismissed login capture doorhanger.

Expected results:
There is a key icon in the URL bar.

Blocks: 1639295
Assignee: nobody → bdanforth
No longer blocks: 1639295
Severity: -- → S2
Status: NEW → ASSIGNED
Priority: -- → P1
Attachment #9150324 - Attachment description: Bug 1638587 - Show dismissed login capture doorhanger when each field is in its own Shadow Root → Bug 1638587 - Show dismissed login capture doorhanger when password field is edited and in its own Shadow Root; r=MattN
Summary: Dismissed login capture doorhanger does not show on edit when each field is in its own Shadow Root → Dismissed login capture doorhanger does not show when password field is edited and in its own Shadow Root
Blocks: 1643163
Attachment #9158680 - Attachment is obsolete: true
Pushed by ncsoregi@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/531f57e501a0
Show dismissed login capture doorhanger when password field is edited and in its own Shadow Root; r=MattN
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla80

QA verification notes

  • This bug covers Case 1 Shadow DOM. All Shadow DOM cases are described in the Description of [META] Bug 1629226 with links to an example page for each case.
  • Guidelines for finding test pages in the wild for all Shadow DOM work are provided in the Feature Technical Documentation for the appropriate Firefox version.
  • It may be challenging to find such pages in the wild, as the prevalence of login pages with Shadow DOM is relatively low (~10%) but growing.
  • This bug's behavior depends on Bug 1612258.

Previous behavior
When editing a password field in a Case 1 Shadow DOM page, the dismissed doorhanger does not show on "input" events.

New behavior
When editing a password field in a Case 1 Shadow DOM page, the dismissed doorhanger shows on "input" events.

Out of scope
This bug only covers edits to the password field, not to the username field, and only for Case 1 Shadow DOM pages. Edits to the username field for Case 1 Shadow DOM and edits to either the password or username field for other Shadow DOM case (i.e. Cases 2 and 3) are out of scope.

Known bugs

  • Bug 1653138: Dismissed login capture doorhanger does not show when editing the password field in VirusTotal login form
    • The root cause lies somewhere upstream of this patch, or it is specific to the VirusTotal page.
  • Bug 1643163: Dismissed login capture doorhanger does not show when username and password fields are edited and each field is in its own Shadow Root
    • Covers edits to the username field (when the password field is non-empty) for Case 1 Shadow DOM pages.
  • Bug 1639295: Dismissed login capture doorhanger does not show on edit when login fields are together in a Shadow Root with a "form" ancestor that is not in a Shadow Root
    • Covers username and password field edits for Case 2 Shadow DOM pages.
Flags: qe-verify+

Unfortunately, I have mixed results based on the tested OS. Tested on the following page:

On Windows 10 both pages are fine, which means edits of the password field are correctly captured and the dismissed doorhanger is displayed.
On MacOS 10.13 and Ubuntu 16.04 it works intermittently, sometimes the dismissed doorhanger appears, other times it doesn't but only on WEGO.com page.

STR to reproduce on Mac/Linux:

  1. Fill in the password field on the login form and check for doorhanger
  2. If it appears, dismiss it in any way (by Don't Save) and reload the page
  3. Repeat step 1-2 multiple times

Wego.com ranks #19,791 in global internet engagement based on Alexa.com so maybe its not worth bothering about intermittent issues on specific OSes for this site. Bianca, can you please share your thoughts about this? I can also submit a bug with logs and recording if you want to check it out but not necessarily block this particular Bug fix from verification.

Flags: needinfo?(bdanforth)

The VirusTotal page is Bug 1653138 -- at least if you're referring to the VirusTotal login form referenced in Bug 1629226. That explains the intermittence as well for that page (as it is related to a race condition). I hope to land a fix for that later today, so can you try it again after that bug lands?

I see also that the Wego login/sign-up page similarly uses Custom Elements (i.e. has a similar set up to the VirusTotal login page in Bug 1653138). It looks to be consistently working when I test the page with my patch from Bug 1653138, so can you try again once it lands?

Flags: needinfo?(bdanforth)

I had no intermittent issues for VirusTotal login form (not including here 1 character password scenario from Bug 1653138), only for Wego.com. However, I checked again on Wego.com after Bug 1653138 landed and I get the dismissed doorhanger for typing a password that is longer than 1 character.
Marking this as verified-fixed for the scenario where the password is longer than 1 character.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Depends on: 1612258
You need to log in before you can comment on or make changes to this bug.