After removed <input type="password"> from the document, history.replaceState() would still cause save login notification being prompted

RESOLVED FIXED in Firefox 66

Status

()

defect
P1
normal
RESOLVED FIXED
2 years ago
5 months ago

People

(Reporter: dbradicich, Assigned: MattN)

Tracking

54 Branch
mozilla66
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 wontfix, firefox56 wontfix, firefox57 wontfix, firefox58 wontfix, firefox59 wontfix, firefox66 fixed)

Details

Attachments

(5 attachments)

Reporter

Description

2 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.115 Safari/537.36

Steps to reproduce:

I had my company's application running and I logged in as admin user using typical form in the app, then after form was gone, used the dev console to call the following

window.history.replaceState({},null,"http://localhost:8081/#browse/browse:maven-releases:com")

Where the previous url was "http://localhost:8081/#browse/browse:maven-releases:com/sonatype"


Actual results:

Each time I call replaceState() I am given the 'Store your credentials' dialog


Expected results:

The state changes with no 'Store your credentials' dialog
Component: Untriaged → DOM
Product: Firefox → Core
Thoughts, Samael?
Flags: needinfo?(sawang)
Is it Firefox? The posted user agent is Chrome 59. And without an accessible public website to reproduce the issue, I really couldn't get clues on what happened.
Flags: needinfo?(sawang)
Reporter

Comment 3

2 years ago
Yes it's firefox, i just happen to use chrome to open the issue ;)

I'll try to give more detail.  

I've got a tree displayed in my app, anytime you click on a node in this tree, we call window.history.replaceState() to update the url with the current tree coordinates.  This all works great, until i authenticate a user.  At this point, anytime i call window.history.replaceState() (whether from our code, or from the console) I get a prompt to save my credentials for the site.  Otherwise everything functions properly (i.e. the replaceState call updates the url as expected)
Do you mean calling replaceState from the page where a login form redirects to? I'm trying to make a minimal dummy test case for it but I still don't quite get it. Does this somewhat look like you're case?

http://freesamael.github.io/gecko/shistory/replaceState/login.html

Could you help me to produce a test case for the issue?
Priority: -- → P3
Reporter

Comment 5

2 years ago
After some more investigation here with other applications, we only have one app that shows the functionality I am noting here.  Which deters me from thinking this is any kind of bug in FF.  I will post an answer here once a resolution was found, but in the mean time, I don't think you guys should spend any more cycles on this
Reporter

Comment 6

2 years ago
(In reply to Damian Bradicich from comment #5)
> After some more investigation here with other applications, we only have one
> app that shows the functionality I am noting here.  Which deters me from
> thinking this is any kind of bug in FF.  I will post an answer here once a
> resolution was found, but in the mean time, I don't think you guys should
> spend any more cycles on this

Just to note, I don't want to close the ticket yet, just do a bit more investigation on my end :)
Reporter

Comment 7

2 years ago
I have opened a port on my router that has our app running, you can access it here http://66.31.253.206:8089/?debug

You will find that prior to signing in, if you do this in the firefox console

window.history.replaceState({}, null, "http://66.31.253.206:8089/#browse:maven-releases:com")

You will see the state change exactly as expected and nothing else happens.

But once you sign in (user/pass -> admin/admin123) and run the same command in the console, you see a store credential dialog popup
Reporter

Comment 8

2 years ago
Is someone able to take a peek at this ?  I'll leave the server up and open over the weekend.

Thanks!

http://66.31.253.206:8089/?debug
(In reply to Damian Bradicich from comment #8)
> Is someone able to take a peek at this ?  I'll leave the server up and open
> over the weekend.
> 
> Thanks!
> 
> http://66.31.253.206:8089/?debug

Thanks Damian. I was stuck at the initializing page last time so I thought you were still working on it. Just realized it might be caused by uBlock origin today.

I'm able to produce the issue with the site you provided, and have a short discussion with a frontend engineer. It seems the sign-in was done by an XHR directly, but not with a <form>. I believe this somehow confuses gecko's LoginManager, that it thinks the replaceState() is where the location changes and should prompt for remembering password. 

I'm trying to make a minimal test with XHR to hopefully make this easier to test.
cc'ing Luke who's more familiar with form autofilling stuff. Damian, could you leave the site alive for a few days more so that people who have better sense with LoginManager have a chance to understand the scenario?
Component: DOM → Form Manager
Product: Core → Toolkit
Minimal test case:

    <input type="text" placeholder="Enter Username"><br>
    <input type="password" placeholder="Enter Password"><br>
    <button onclick="history.replaceState({}, 'replaceState causes password prompt', location.href);">replaceState</button>
Summary: window.history.replaceState() causes authentication storage dialog → history.replaceState() in a page with <input type="password"> causes save login notification being prompted
Reporter

Comment 12

2 years ago
Great, glad you were able to reproduce, yes sure, i'll do my best to leave the site up for the foreseeable future.  

Note that you can simply do http://66.31.253.206:8089/ for faster load times, you just have the ?debug param in case debugging any of our (or extjs code) proves handy.

Thanks!
This should be a better example than comment 11: http://freesamael.github.io/gecko/password-notification/replaceState/login.html

(and BTW Safari has the same symptom)
(In reply to Samael Wang [:freesamael] from comment #11)
> Minimal test case:

This minimal test case is demonstrating expected behaviour. replaceState is supposed to cause a login prompt if there are login fields present.

What is the actual problem you are encountering since running replaceState from the console when login fields are present is expected to show the prompt? Are you leaving hidden login fields (e.g. type=password) in the document even after login?
Blocks: 1166947
Component: Form Manager → Password Manager
Flags: needinfo?(dbradicich)
(In reply to Matthew N. [:MattN] (huge backlog; PM if requests are blocking you) from comment #14)
> (In reply to Samael Wang [:freesamael] from comment #11)
> > Minimal test case:
> 
> This minimal test case is demonstrating expected behaviour. replaceState is
> supposed to cause a login prompt if there are login fields present.
> 
> What is the actual problem you are encountering since running replaceState
> from the console when login fields are present is expected to show the
> prompt? Are you leaving hidden login fields (e.g. type=password) in the
> document even after login?

Yes. I later realized that, so I provided another test case in comment 13 which should more resemble what Damian is encountering:

<html lang="en">
  <head>
    <meta charset="utf-8">
    <title>Test LoginManager</title>
    <script type="application/javascript">
    function doFakeSignIn() {
      // (assume there're XHR operations here to post credentials...)
      // ...
      document.querySelector("#fake-login-form").remove();
      document.querySelector("#text").textContent = "You're signed in";
    }
    </script>
  </head>
  <body>
    <div id="fake-login-form">
      <input type="text" placeholder="Enter Username"><br>
      <input type="password" placeholder="Enter Password"><br>
      <button onclick="doFakeSignIn();">SignIn</button>
    </div>
    <p id="text"></p>
    <button onclick="history.replaceState({}, 'replaceState causes password prompt', location.href);">replaceState</button>
  </body>
</html>

It's that even if the script completely remove input fields from the document, history.replaceState() would still cause a prompt. I should have updated the summary. My bad.
Summary: history.replaceState() in a page with <input type="password"> causes save login notification being prompted → After removed <input type="password"> from the document, history.replaceState() woudl still cause save login notification being prompted
Summary: After removed <input type="password"> from the document, history.replaceState() woudl still cause save login notification being prompted → After removed <input type="password"> from the document, history.replaceState() would still cause save login notification being prompted
Reporter

Comment 16

2 years ago
Are you still expecting anything from me ?  (I see issue is still marked as 'needs info'
Flags: needinfo?(dbradicich)
Duplicate of this bug: 1406945
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: P3 → P1
Moving to p3 because no activity for at least 24 weeks.
Priority: P1 → P3
Reporter

Comment 20

8 months ago
just to note, i've just been waiting for the issue to be picked up, still just as high priority to me as ever, figured pinging for updates would get annoying :)
Yeah, this is still high-priority and shouln't have been changed.
Priority: P3 → P1
Attachment #9034872 - Attachment filename: file_1386283.txt → file_1386283.html
Attachment #9034872 - Attachment mime type: text/plain → text/html

(In reply to Damian Bradicich from comment #20)

just to note, i've just been waiting for the issue to be picked up, still
just as high priority to me as ever, figured pinging for updates would get
annoying :)

Hi Damian,

It seems like bug 1456843 (released in Firefox 63 on 2018-10-23) has fixed the testcase so the problem no longer occurs. Are you still seeing the issue?

If so, could you provide more details, maybe making the test page available again or attaching a reduced testcase like comment 22 that causes the problem.

Flags: needinfo?(dbradicich)
Reporter

Comment 25

5 months ago

(In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from comment #23)

(In reply to Damian Bradicich from comment #20)

just to note, i've just been waiting for the issue to be picked up, still
just as high priority to me as ever, figured pinging for updates would get
annoying :)

Hi Damian,

It seems like bug 1456843 (released in Firefox 63 on 2018-10-23) has fixed the testcase so the problem no longer occurs. Are you still seeing the issue?

If so, could you provide more details, maybe making the test page available again or attaching a reduced testcase like comment 22 that causes the problem.

Hi Matthew, so I went ahead and tried to reproduce, and it looks like all is well now :) Thanks for the heads up!

Flags: needinfo?(dbradicich)

Great, thanks for confirming! I'm still going to do a more proper fix (and add a test) since bug 1456843 didn't really intend to fix this.

Assignee: nobody → MattN+bmo
Status: NEW → ASSIGNED
Attachment #9034875 - Attachment description: Bug 1386283 - Only consider connected <input>s for username/password fields. → Bug 1386283 - Only consider connected <input>s for username/password fields. r=mconley

Comment 30

5 months ago
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/autoland/rev/d5357c40725c
Only consider connected <input>s for username/password fields. r=mconley
https://hg.mozilla.org/integration/autoland/rev/c39b7fb8bbdd
Test: Only consider connected <input>s for username/password fields. r=mconley
https://hg.mozilla.org/integration/autoland/rev/06cb65fac0f3
Enable eslint 'indent' rule on passwordmgr with --fix. r=mconley
https://hg.mozilla.org/integration/autoland/rev/16ce9a71ff04
Enable eslint 'brace-style' and 'curly' rules on passwordmgr with --fix. r=mconley
See Also: → 1519304
You need to log in before you can comment on or make changes to this bug.