Closed Bug 1251916 Opened 4 years ago Closed 4 years ago

Use standard JavaScript features in toolkit/components/passwordmgr to pass eslint checks

Categories

(Toolkit :: Password Manager, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: aryx, Assigned: aryx)

Details

Attachments

(5 files)

58 bytes, text/x-review-board-request
MattN
: review+
Details
58 bytes, text/x-review-board-request
MattN
: review+
Details
58 bytes, text/x-review-board-request
MattN
: review+
Details
58 bytes, text/x-review-board-request
MattN
: review+
Details
58 bytes, text/x-review-board-request
MattN
: review+
Details
No description provided.
While it doesn't seem like this SpiderMonkey feature will go away soon, replacing it makes tooling easier and is suitable here because the unconditional catch js already used.

Review commit: https://reviewboard.mozilla.org/r/37071/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/37071/
Attachment #8724526 - Flags: review?(MattN+bmo)
No bug yet - enable eslint for toolkit/components/passwordmgr. try: -b do -p macosx64,win32,android-api-15 -u crashtest,crashtest-e10s,xpcshell,luciddream,mochitests,mochitest-1,mochitest-2,mochitest-3,mochitest-4,mochitest-5,mochitest-6,mochitest-7,mochitest-8,mochitest-9,mochitest-10,mochitest-11,mochitest-12,mochitest-13,mochitest-14,mochitest-15,mochitest-16,mochitest-gl,crashtest-1,crashtest-2,xpcshell-1,xpcshell-2,xpcshell-3,mochitest-chrome,crashtest-1,crashtest-2,crashtest-3 -t none

Review commit: https://reviewboard.mozilla.org/r/37075/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/37075/
Attachment #8724528 - Flags: review?(MattN+bmo)
Comment on attachment 8724524 [details]
MozReview Request: Bug 1251916 - Use standard JavaScript features in toolkit/components/passwordmgr to pass eslint checks: general, simple changes. r?MattN

https://reviewboard.mozilla.org/r/37067/#review33625

::: toolkit/components/passwordmgr/LoginManagerContent.jsm:1337
(Diff revision 1)
> -      elements: [for (el of doc.documentElement.querySelectorAll("input")) if (!el.form) el],
> +      elements: elements,

Nit: `elements,` is sufficient with newer syntax

::: toolkit/components/passwordmgr/test/test_privbrowsing_perwindowpb.html:158
(Diff revision 1)
>      case 7:
>        // verify that the user/pass pair was autofilled
> -      var gotUser = iframe.contentDocument.getElementById("user").textContent;
> -      var gotPass = iframe.contentDocument.getElementById("pass").textContent;
> +      let gotUser = iframe.contentDocument.getElementById("user").textContent;
> +      let gotPass = iframe.contentDocument.getElementById("pass").textContent;
>        is(gotUser, "notifyu1", "Checking submitted username");

Remove the `let`s here for consistency and use the added `var` above.
Attachment #8724524 - Flags: review?(MattN+bmo) → review+
Comment on attachment 8724525 [details]
MozReview Request: Bug 1251916 - Use standard JavaScript features in toolkit/components/passwordmgr to pass eslint checks: replace legacy generators with ES6 generators. r?MattN

https://reviewboard.mozilla.org/r/37069/#review33627

::: toolkit/components/passwordmgr/test/LoginTestUtils.jsm:39
(Diff revision 1)
> -  reloadData() {
> +  * reloadData() {

Hmm… I didn't know about this syntax as I've never seen it used in m-c. I always used the more verbose syntax for methods when I needed a generator. Good to know.
Attachment #8724525 - Flags: review?(MattN+bmo) → review+
Attachment #8724526 - Flags: review?(MattN+bmo) → review+
Comment on attachment 8724526 [details]
MozReview Request: Bug 1251916 - Use standard JavaScript features in toolkit/components/passwordmgr to pass eslint checks: replace conditional catch statements. r?MattN

https://reviewboard.mozilla.org/r/37071/#review33629
Attachment #8724527 - Flags: review?(MattN+bmo) → review+
Comment on attachment 8724527 [details]
MozReview Request: Bug 1251916 - Use standard JavaScript features in toolkit/components/passwordmgr to pass eslint checks: ignore statements which eslint regards as native reassignments. r?MattN

https://reviewboard.mozilla.org/r/37073/#review33631
Comment on attachment 8724528 [details]
MozReview Request: Bug 1251916 - Use standard JavaScript features in toolkit/components/passwordmgr to pass eslint checks: enable eslint. r?MattN

https://reviewboard.mozilla.org/r/37075/#review33633

Nit: please remove the cruft from the end of this commit message

Also, please land this ASAP if try is good since I'm rewriting tests for e10s and am touching a lot of test files as well.
Attachment #8724528 - Flags: review?(MattN+bmo) → review+
You need to log in before you can comment on or make changes to this bug.