Enable eslint no-undef for browser/, apart from components/ and base/content/

RESOLVED FIXED in Firefox 54

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

unspecified
Firefox 54
Points:
---

Firefox Tracking Flags

(firefox54 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Assignee)

Description

2 years ago
We're now in a position that we can enable no-undef for some of browser/ - everything bar components/ and base/content/. Those directories still need a bit of work for common failures.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 3

2 years ago
mozreview-review
Comment on attachment 8832855 [details]
Bug 1336070 - Enable eslint no-undef for browser/, apart from components/ and base/content/.

https://reviewboard.mozilla.org/r/109098/#review110282

::: tools/lint/eslint/eslint-plugin-mozilla/lib/rules/import-browserjs-globals.js:63
(Diff revision 1)
> -          !helpers.getIsHeadFile(this)) {
> +      let root = helpers.getRootDir(filepath);
> +      let relativepath = path.relative(root, filepath);
> +
> +      if ((helpers.getTestType(this) != "browser" &&
> +          !helpers.getIsHeadFile(this)) &&
> +          !relativepath.includes("content")) {

This starts to open the rule up to content files (in this case, for browser/extensions/pocket/content/main.js).

I'm not convinced yet that this is fully the right approach, but I'll know more with the next step of getting some of the content directories passing.

Comment 4

2 years ago
mozreview-review
Comment on attachment 8832854 [details]
Bug 1336070 - Fix no-undef issues in the pocket code.

https://reviewboard.mozilla.org/r/109096/#review110344

::: browser/extensions/pocket/content/panels/js/saved.js:177
(Diff revision 1)
>              },
>              textToData(text) {
>                  if ($.trim(text).length > 25 || !$.trim(text).length) {
>                      if (text.length > 25) {
>                          myself.showTagsError(myself.dictJSON.maxtaglength);
> -                        changestamp = Date.now();
> +                        window.changestamp = Date.now();

Does this need to be on window?  Can it be on myself?
Attachment #8832854 - Flags: review?(mixedpuppy) → review+
cc/mkaply on pocket stuff

Comment 6

2 years ago
mozreview-review
Comment on attachment 8832855 [details]
Bug 1336070 - Enable eslint no-undef for browser/, apart from components/ and base/content/.

https://reviewboard.mozilla.org/r/109098/#review110350
Attachment #8832855 - Flags: review?(dtownsend) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 9

2 years ago
mozreview-review-reply
Comment on attachment 8832854 [details]
Bug 1336070 - Fix no-undef issues in the pocket code.

https://reviewboard.mozilla.org/r/109096/#review110344

> Does this need to be on window?  Can it be on myself?

After re-evaluating, it doesn't look like it does. Changed to `this`.

Comment 10

2 years ago
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c34aa5230729
Fix no-undef issues in the pocket code. r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/fc7f502a43a1
Enable eslint no-undef for browser/, apart from components/ and base/content/. r=mossop
Backed out for failing browser_button_state.js and browser_report_site_issue.js:

https://hg.mozilla.org/integration/autoland/rev/ed1c85a67cdac1af3edb0578c9d311023ac0b7a1
https://hg.mozilla.org/integration/autoland/rev/fd0fc3404d168cc62d9e83d24b8999fc92278df3

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=fc7f502a43a10c0f19f700a33c5d31c6c455bebb
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=74261536&repo=autoland

[task 2017-02-03T12:14:50.469178Z] 12:14:50     INFO - TEST-START | browser/extensions/webcompat-reporter/test/browser/browser_button_state.js
[task 2017-02-03T12:14:51.546781Z] 12:14:51     INFO - TEST-INFO | started process screentopng
[task 2017-02-03T12:14:53.099868Z] 12:14:53     INFO - TEST-INFO | screentopng: exit 0
[task 2017-02-03T12:14:53.100342Z] 12:14:53     INFO - Buffered messages logged at 12:14:50
[task 2017-02-03T12:14:53.106912Z] 12:14:53     INFO - Entering test bound test_button_state_disabled
[task 2017-02-03T12:14:53.107199Z] 12:14:53     INFO - Buffered messages finished
[task 2017-02-03T12:14:53.108741Z] 12:14:53     INFO - TEST-UNEXPECTED-FAIL | browser/extensions/webcompat-reporter/test/browser/browser_button_state.js | Check that button is enabled for reportable schemes on tab load - Got true, expected false
[task 2017-02-03T12:14:53.108983Z] 12:14:53     INFO - Stack trace:
[task 2017-02-03T12:14:53.109282Z] 12:14:53     INFO -     chrome://mochikit/content/browser-test.js:test_is:911
[task 2017-02-03T12:14:53.110736Z] 12:14:53     INFO -     chrome://mochitests/content/browser/browser/extensions/webcompat-reporter/test/browser/browser_button_state.js:test_button_state_disabled:10
[task 2017-02-03T12:14:53.111123Z] 12:14:53     INFO -     Tester_execTest@chrome://mochikit/content/browser-test.js:735:9
[task 2017-02-03T12:14:53.113380Z] 12:14:53     INFO -     Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:655:7
[task 2017-02-03T12:14:53.114801Z] 12:14:53     INFO -     SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:744:59
[task 2017-02-03T12:14:53.11781
Flags: needinfo?(standard8)
Comment hidden (mozreview-request)
(Assignee)

Comment 13

2 years ago
I've just fixed up the test failures, which were due to a scoping issue. Only minor corrections, so pushing again.
Flags: needinfo?(standard8)

Comment 14

2 years ago
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/40e4945412fe
Fix no-undef issues in the pocket code. r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/bd99ca8c230b
Enable eslint no-undef for browser/, apart from components/ and base/content/. r=mossop

Comment 15

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/40e4945412fe
https://hg.mozilla.org/mozilla-central/rev/bd99ca8c230b
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
You need to log in before you can comment on or make changes to this bug.