As part of rolling out ESLint across the tree, we should enable it for dom/security/test/unit/ There's background on our eslint setups here: https://developer.mozilla.org/docs/ESLint Here's some approximate steps: - In .eslintignore, remove the `dom/security/test/unit/**` line. - Run eslint: ./mach eslint --fix dom/security/test/unit/ This should fix most/all of the issues. - Inspect the diff to make sure that the indentation of the lines surrounding the changes look ok. - Create a commit of the work so far. - For the remaining issues, you'll need to fix them by hand, please ask if you need help. (you can just run `./mach eslint dom/security/test/unit/` to check you've fixed them). - Create a second commit of the manual changes and push it to mozreview: http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview.html If you're not sure of how to fix something, please look at the rules documentation http://eslint.org/docs/rules/ or just ask here.

Comment on attachment 8938649 [details] Bug 1423844 Enable ESLint for dom/security/test/unit/ https://reviewboard.mozilla.org/r/209248/#review215110 Thanks for the patch, it looks good. There's a couple of things we can tidy up further though. When you upload the patch, please can you also request additional review from ckerschb, who is owner of this area. ::: dom/security/test/unit/test_csp_reports.js:89 (Diff revision 1) > > dump("Created test " + id + " : " + policy + "\n\n"); > > - let ssm = Cc["@mozilla.org/scriptsecuritymanager;1"] > + let ssm = Services.scriptSecurityManager; > - .getService(Ci.nsIScriptSecurityManager); > principal = ssm.createCodebasePrincipal(selfuri, {}); We can drop the intermidate variable for ssm here, it is only used once. ::: dom/security/test/unit/test_csp_upgrade_insecure_request_header.js:10 (Diff revision 1) > Cu.import("resource://testing-common/httpd.js"); > Cu.import("resource://gre/modules/NetUtil.jsm"); > Cu.import("resource://gre/modules/XPCOMUtils.jsm"); > > -var prefs = Cc["@mozilla.org/preferences-service;1"]. > - getService(Ci.nsIPrefBranch); > +Cu.import("resource://gre/modules/Services.jsm"); > +var prefs = Services.prefs; prefs is only used in one place, so might as well drop the intermediate variable and use Services.prefs.setBoolPref directly below. ::: dom/security/test/unit/test_isOriginPotentiallyTrustworthy.js:25 (Diff revision 1) > "@mozilla.org/contentsecuritymanager;1", > "nsIContentSecurityManager"); > > -var prefs = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch); > + > +var prefs = Services.prefs; > prefs.setCharPref("dom.securecontext.whitelist", "example.net,example.org"); Ditto here - drop the intermidiate variable.

Opening as a mentored bug. There's a previous patch here, though that's a bit out of date and incomplete (see comment 2): https://hg.mozilla.org/mozreview/gecko/rev/0122a8570972a2987990ecfb0447eda0d367a76d What is probably easiest to do is to generally follow comment 0: - Run the automatic fix option, and manually fix the remaining issues. See also comment 2. - There's not many failures here, so I think it is ok to end up with just one commit rather than two. - Posting a patch now should be performed via phabricator: https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html

Hi , I would like to work on it , can you please guide me how to go ahead with it? Thanks

Sure, please see my comment 3 as the best way to get started on this.

Shivam Singhal are you still working on this bug?

Hi @Ferenc , yes i am on it , will raise a patch soon :)

Hi @Mark , can you help me with phabricator please, while pushing the changes I am getting this error https://pastebin.com/636W2JHj . I made the changes as well https://pastebin.com/hwpGUzHc but I am unable to push the changes

Enable ESLint for dom/security/test/unit/

Enable ESLint for dom/security/test/unit

I guess you figured out your issues, my best guess would have been that you were using the wrong user to run the command (or some of your files had the wrong permissions). For some reason there's two commits here which as far as I can tell are exactly the same, so I'll obsolete the older one in a moment. Normally, per comment 0, I'd expect two commits - one for the automatic changes, one for the manual. However, in this case there's so few changes that we don't need to do that, we'll proceed with just the one.

Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8df1cbc7c991 Enable ESLint for dom/security/test/unit/ r=Standard8,jkt

(In reply to Mark Banner (:standard8) from comment #11) > I guess you figured out your issues, my best guess would have been that you > were using the wrong user to run the command (or some of your files had the > wrong permissions). Hi @Mark, yes by mistake the project was on root of my laptop due to that,I was getting problem in pushing the changes. > > For some reason there's two commits here which as far as I can tell are > exactly the same, so I'll obsolete the older one in a moment. Yeah I did not know that when they get pushed , I was trying regularly to push the changes. I am not that much used to `arc` and `phabricator`. > > Normally, per comment 0, I'd expect two commits - one for the automatic > changes, one for the manual. However, in this case there's so few changes > that we don't need to do that, we'll proceed with just the one. Sorry I will keep in my mind from next time. Thanks for the help.

Hi Shivam, no problem. The patch has now landed on our integration branch, assuming no problems are detected with the tests, it will land in the main branch within the next 24 hours (when this bug will also get marked as fixed). Thank you for working on this.