Closed
Bug 1257757
Opened 8 years ago
Closed 8 years ago
Include filenames in InsecurePasswordUtils errors
Categories
(Toolkit :: Password Manager, defect, P1)
Toolkit
Password Manager
Tracking
()
Tracking | Status | |
---|---|---|
firefox48 | --- | verified |
People
(Reporter: MattN, Assigned: MattN)
References
(Blocks 1 open bug)
Details
(Whiteboard: [fxprivacy])
Attachments
(1 file)
Currently the errors say "(unknown)" where the filename is supposed to show which makes it harder for developers to figure out what's causing the warning.
Assignee | ||
Comment 1•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/40973/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/40973/
Attachment #8732027 -
Flags: review?(bgrinstead)
Assignee | ||
Comment 2•8 years ago
|
||
There are no tests for this module yet but Sean will add some in bug 1191092.
Updated•8 years ago
|
Flags: qe-verify?
Whiteboard: [fxprivacy] [triage] → [fxprivacy]
Updated•8 years ago
|
Attachment #8732027 -
Flags: review?(bgrinstead) → review+
Comment 3•8 years ago
|
||
Comment on attachment 8732027 [details] MozReview Request: Bug 1257757 - Add filenames to insecure password warnings. r=bgrins https://reviewboard.mozilla.org/r/40973/#review37647 ::: toolkit/components/passwordmgr/InsecurePasswordUtils.jsm:32 (Diff revision 1) > let category = "Insecure Password Field"; > // All web console messages are warnings for now. > let flag = Ci.nsIScriptError.warningFlag; > let message = l10n.getStr(messageTag); > let consoleMsg = Cc["@mozilla.org/scripterror;1"].createInstance(Ci.nsIScriptError); > - consoleMsg.initWithWindowID(message, "", 0, 0, 0, flag, category, windowId); > + consoleMsg.initWithWindowID(message, domDoc.location.href, 0, 0, 0, flag, category, windowId); It looks like most places use the file name only: https://dxr.mozilla.org/mozilla-central/search?q=initWithWindowID&redirect=true&case=false But it seems fine to use href (any string will do)
Assignee | ||
Comment 4•8 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #3) > It looks like most places use the file name only: > https://dxr.mozilla.org/mozilla-central/ > search?q=initWithWindowID&redirect=true&case=false > > But it seems fine to use href (any string will do) I thought I checked and e.fileName has the whole URL. I want it so that clicking on the file name opens to the source which it currently does with this patch. I guessed that only the filename wouldn't be able to do that.
Comment 5•8 years ago
|
||
(In reply to Matthew N. [:MattN] (behind on bugmail) from comment #4) > (In reply to Brian Grinstead [:bgrins] from comment #3) > > It looks like most places use the file name only: > > https://dxr.mozilla.org/mozilla-central/ > > search?q=initWithWindowID&redirect=true&case=false > > > > But it seems fine to use href (any string will do) > > I thought I checked and e.fileName has the whole URL. I want it so that > clicking on the file name opens to the source which it currently does with > this patch. I guessed that only the filename wouldn't be able to do that. Do you have a good test page / STR for this error to show up?
Comment 7•8 years ago
|
||
Disregard comment 5.. I have a test page at http://bgrins.github.io/devtools-demos/inspector/form.html
Comment 8•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5ed6d4e80064
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Updated•8 years ago
|
Iteration: --- → 48.2 - Apr 4
Priority: -- → P1
Updated•8 years ago
|
Flags: qe-verify? → qe-verify+
QA Contact: paul.silaghi
You need to log in
before you can comment on or make changes to this bug.
Description
•