1343945 - Fix eslint errors in browser/base/content/browser-social.js

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Sam Foster [:sfoster] (he/him)]

browser/base/content/browser-social.js is in the global .estlintignore file, and needs a little tidy-up to remove the exclusion.

Comment 1

[Sam Foster [:sfoster] (he/him)]

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?

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

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)

Comment 3

[Sam Foster [:sfoster] (he/him)]

Attachment: Bug 1343945 - Fix lint errors from browser-social.js, remove from ignore list

* Bug 1343945 - Fix lint errors, remove from ignore list
* Bug 1343945 - Fix lint errors, remove from ignore list. r?jaws
* Collapsed revision
* Bug 1343945 - Fix lint errors and remove from ignore list

* Bug 1343945 - Fix lint errors, remove from ignore list

* Fixup

Review commit: https://reviewboard.mozilla.org/r/116864/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/116864/

Comment 4

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8843104 [details]
Bug 1343945 - Fix lint errors from browser-social.js, remove from ignore list

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116864/diff/1-2/

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

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;

Comment 6

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8843104 [details]
Bug 1343945 - Fix lint errors from browser-social.js, remove from ignore list

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116864/diff/2-3/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/15f2040d22ff
Fix lint errors from browser-social.js, remove from ignore list r=jaws

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/15f2040d22ff