Closed Bug 1527828 Opened 4 years ago Closed 3 years ago

Remove insecure password field detection code for the address bar


(Toolkit :: Password Manager, enhancement, P2)




Performance Impact medium
Fission Milestone M4
Tracking Status
firefox71 --- fixed


(Reporter: MattN, Assigned: MattN)



(Keywords: perf, perf:pageload, Whiteboard: [fxperf:p3])


(2 files)

_detectInsecureFormLikes[1] runs on every pageshow and from the DOMFormHasPassword event and shows up in this profile from smaug:

It's also not going to work properly with Fission IIUC since it traverses window.frames.

IMO this logic which is mostly independent of password manager should be rewritten in C++ and work more like the mixed content address bar indicator. That should provide page load speed improvements.

Test pages:

You should see a broken lock icon in the address bar AND the identity panel should say "Logins entered on this page could be compromised" inside.
I don't have a test-page with subframes handy but you can probably embed those test pages in an <iframe> on an http: server.


On a related note, the insecure login field console warnings from InsecurePasswordUtils.jsm could also be consolidated with the address bar indicator logic and we could avoid loading that JSM on page load too. There is also telemetry being accumulated on every page load in InsecurePasswordUtils which could have a perf impact.

Whiteboard: [qf][fxperf] → [qf:p2:pageload][fxperf]
Whiteboard: [qf:p2:pageload][fxperf] → [qf:p2:pageload][fxperf:p3]
Fission Milestone: ? → Future

In bug 1555317 comment 27 we are discussing whether we can now remove this code altogether… doing so could help pageload performance and then we wouldn't have to fix it for Fission.

See Also: → 1555317

We got the go-ahead to remove this code now (bug 1555317 comment 28) so we don't have to fix it for Fission and we can reduce the work we do on page loads and tab switches.

Summary: Move insecure password field detection for the address bar indicator to C++ and fix it for Fission → Remove insecure password field detection code for the address bar

So is this a dupe of bug 1567827 now? :)

Maybe, this bug has the context for performance and focused on the passwordmgr/ content process portion whereas bug 1567827 is more about the doorhanger parent process portion. They can be done in one bug or separate depending on the dev. who takes it.

Blocks: 1567827
Blocks: 1582112

Deleting this code allows us to delete 3 failing tests in Fission so moving to M4.

Assignee: nobody → MattN+bmo
Fission Milestone: Future → M4
Attachment #9100961 - Attachment mime type: text/plain → text/html
Pushed by
Remove insecure password field detection code for the address bar. r=sfoster
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
Flags: qe-verify+
Performance Impact: --- → P2
Keywords: perf:pageload
Whiteboard: [qf:p2:pageload][fxperf:p3] → [fxperf:p3]
You need to log in before you can comment on or make changes to this bug.