1651186 - Entire file: URI is now used as the origin for password manager

Status: RESOLVED FIXED

(Toolkit :: Password Manager, defect, P1)

Description

[Matthew N. [:MattN]]

We may want to make this change in the future for bug 1625391 but we would want to do more to ease the transition. For now let's not regress the previous behaviour unintentionally (since LoginHelper.getLoginOrigin would be inconsistent with the new behaviour in LoginManagerParent).

See bug 1625391 comment 8.

Comment 1

[Matthew N. [:MattN]]

Attachment: Bug 1651186 - Use getLoginOrigin to remove the path from file: URIs for pwmgr. r=severin

Comment 2

[Matthew N. [:MattN]]

Comment on attachment 9161981 [details]
Bug 1651186 - Use getLoginOrigin to remove the path from file: URIs for pwmgr. r=severin

Beta/Release Uplift Approval Request

• User impact if declined: Logins saved for file:// will no longer be filled file URIs. Upon submitting a file: URI form, the password manager will ask to save a new login even if the login was already saved.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Use a widely-used helper to sanitize an origin for the password manager
• String changes made/needed: None

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3a38af2b7e83
Use getLoginOrigin to remove the path from file: URIs for pwmgr. r=severin

Comment 4

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out changeset 3a38af2b7e83 (bug 1651186) for browser_password_management.js failures

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=mochitest-browser-chrome-e10s-&fromchange=4a732e51e60413082df5551fa9fe9d556cbfc04a&tochange=34fb169ef962bb4c06b4436f78b91d94e0e90f4b&selectedTaskRun=XFTfFVY_TeaxK2Ggtk6DsA.0

Backout link: https://hg.mozilla.org/integration/autoland/rev/34fb169ef962bb4c06b4436f78b91d94e0e90f4b

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=308932338&repo=autoland&lineNumber=20476

...
[task 2020-07-08T05:07:17.784Z] 05:07:17     INFO - GECKO(6188) | [Parent 2644, Jump List] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80520012 (NS_ERROR_FILE_NOT_FOUND): file /builds/worker/checkouts/gecko/widget/windows/WinUtils.cpp, line 1360
[task 2020-07-08T05:08:11.911Z] 05:08:11     INFO - TEST-INFO | started process screenshot
[task 2020-07-08T05:08:11.976Z] 05:08:11     INFO - TEST-INFO | screenshot: exit 0
[task 2020-07-08T05:08:11.976Z] 05:08:11     INFO - Buffered messages logged at 05:06:41
[task 2020-07-08T05:08:11.976Z] 05:08:11     INFO - Entering test bound test_openPasswordManagement
[task 2020-07-08T05:08:11.976Z] 05:08:11     INFO - Buffered messages logged at 05:06:44
[task 2020-07-08T05:08:11.976Z] 05:08:11     INFO - TEST-PASS | browser/components/preferences/tests/browser_password_management.js | Save Password CheckBox should be unchecked by default - true == true - 
[task 2020-07-08T05:08:11.976Z] 05:08:11     INFO - Console message: [JavaScript Error: "Error: An origin is required" {file: "resource://gre/modules/LoginManagerParent.jsm" line: 237}]
[task 2020-07-08T05:08:11.976Z] 05:08:11     INFO - receiveMessage@resource://gre/modules/LoginManagerParent.jsm:237:13
[task 2020-07-08T05:08:11.976Z] 05:08:11     INFO - 
[task 2020-07-08T05:08:11.977Z] 05:08:11     INFO - Buffered messages finished
[task 2020-07-08T05:08:11.977Z] 05:08:11     INFO - TEST-UNEXPECTED-FAIL | browser/components/preferences/tests/browser_password_management.js | Test timed out - 
[task 2020-07-08T05:08:11.977Z] 05:08:11     INFO - GECKO(6188) | MEMORY STAT | vsize 2104965MB | vsizeMaxContiguous 65290747MB | residentFast 566MB | heapAllocated 118MB
[task 2020-07-08T05:08:11.977Z] 05:08:11     INFO - TEST-OK | browser/components/preferences/tests/browser_password_management.js | took 90105ms
[task 2020-07-08T05:08:11.977Z] 05:08:11     INFO - Not taking screenshot here: see the one that was previously logged
[task 2020-07-08T05:08:11.977Z] 05:08:11     INFO - TEST-UNEXPECTED-FAIL | browser/components/preferences/tests/browser_password_management.js | Found a tab after previous test timed out: about:preferences#privacy - 
[task 2020-07-08T05:08:12.017Z] 05:08:12     INFO - GECKO(6188) | [Parent 2644, Main Thread] WARNING: '!inner', file /builds/worker/checkouts/gecko/dom/ipc/JSWindowActorProtocol.cpp, line 172
[task 2020-07-08T05:08:12.017Z] 05:08:12     INFO - GECKO(6188) | [Parent 2644, Main Thread] WARNING: '!inner', file /builds/worker/checkouts/gecko/dom/ipc/JSWindowActorProtocol.cpp, line 172
[task 2020-07-08T05:08:12.035Z] 05:08:12     INFO - GECKO(6188) | [Child 4532: Main Thread]: I/DocShellAndDOMWindowLeak ++DOCSHELL 0000023DB4D28800 == 1 [pid = 4532] [id = {e8f3541f-fdb2-40d3-9be8-d14d9d3d69e9}]
[task 2020-07-08T05:08:12.035Z] 05:08:12     INFO - GECKO(6188) | [Child 4532: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 1 (0000023DB49D0090) [pid = 4532] [serial = 145] [outer = 0000000000000000]
[task 2020-07-08T05:08:12.035Z] 05:08:12     INFO - GECKO(6188) | [Child 4532: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 2 (0000023DB4DB8800) [pid = 4532] [serial = 146] [outer = 0000023DB49D0090]
[task 2020-07-08T05:08:12.061Z] 05:08:12     INFO - GECKO(6188) | [Child 4532: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 3 (0000023DB4DC1000) [pid = 4532] [serial = 147] [outer = 0000023DB49D0090]
[task 2020-07-08T05:08:12.066Z] 05:08:12     INFO - checking window state
...

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1c9726711f76
Use getLoginOrigin to remove the path from file: URIs for pwmgr. r=severin

Comment 6

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/1c9726711f76

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9161981 [details]
Bug 1651186 - Use getLoginOrigin to remove the path from file: URIs for pwmgr. r=severin

Thanks for including a test. Approved for 79.0b8.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/11cdca9ca9ac