Closed Bug 1343945 Opened 3 years ago Closed 3 years ago
Fix eslint errors in browser/base/content/browser-social
59 bytes, text/x-review-board-request
browser/base/content/browser-social.js is in the global .estlintignore file, and needs a little tidy-up to remove the exclusion.
Not sure of the component for this. Linting without the no-undef rule is straight-forward. To pass with no-undef I'll need some pointers on how to handle the many globals it uses?
Assignee: nobody → sfoster
https://developer.mozilla.org/en-US/docs/ESLint#no-undef should cover most of this. Over IRC Standard8 said: > sfoster: at a glance, I think you could add /* eslint-env mozilla/browser-window */ to the top of the file and be most of the way there (today’s tree you’ll need)
Attachment #8843104 - Flags: review?(jaws)
Comment on attachment 8843104 [details] Bug 1343945 - Fix lint errors from browser-social.js, remove from ignore list https://reviewboard.mozilla.org/r/116864/#review118678 ::: commit-message-c54c6:3 (Diff revision 2) > +Bug 1343945 - Fix lint errors, remove from ignore list > +* Added no-undef rule locally as we're not ready to turn on for the whole directory > +* Adding "use strict"; inside the iife causes an exception - I'm not sure how to see it? So the "use strict" exception is because in a "use strict" anonymous function, `this` is undefined. See http://stackoverflow.com/questions/9822561/why-is-this-in-an-anonymous-function-undefined-when-using-strict The simplest way to fix this is by using .call(this) at the bottom of the file, like so: > -})(); > +}).call(this); As to "how to see it", I don't know of a good way. I commented out the body of SocialUI and SocialShare and still saw the error, so that left me with only about 10 lines of code that the issue could have been in. ::: browser/base/content/browser-social.js:403 (Diff revision 2) > // graphData is an optional param that either defines the full set of data > // to be shared, or partial data about the current page. It is set by a call > // in mozSocial API, or via nsContentMenu calls. If it is present, it MUST > // define at least url. If it is undefined, we're sharing the current url in > // the browser tab. > - let pageData = graphData ? graphData : this.currentShare; > + let sharedPageData = graphData ? graphData : this.currentShare; Might as well change this line to: let sharedPageData = graphData || this.currentShare;
Attachment #8843104 - Flags: review?(jaws) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/15f2040d22ff Fix lint errors from browser-social.js, remove from ignore list r=jaws
You need to log in before you can comment on or make changes to this bug.