Closed Bug 1177774 Opened 9 years ago Closed 9 years ago

Fix remaining eslint issues given current config

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(firefox41 fixed)

RESOLVED FIXED
Firefox 41
Tracking Status
firefox41 --- fixed

People

(Reporter: mcomella, Assigned: mcomella)

References

Details

Attachments

(9 files)

(deleted), text/x-review-board-request
Margaret
: review+
Details
(deleted), text/x-review-board-request
Margaret
: review+
Details
(deleted), text/x-review-board-request
Margaret
: review+
Details
(deleted), text/x-review-board-request
mcomella
: review+
Margaret
: review+
nalexander
: review+
Details
(deleted), text/x-review-board-request
Margaret
: review+
Details
(deleted), text/x-review-board-request
Margaret
: review+
Details
(deleted), text/x-review-board-request
Margaret
: review+
Details
(deleted), text/x-review-board-request
Margaret
: review+
Details
(deleted), text/x-review-board-request
Margaret
: review+
Details
When running `pushd mobile/android && eslint . --ext "[.js,.jsm]; popd"`. That way we can start failing when it fails. via bug 1170804 comment 20: > (In reply to Michael Comella (:mcomella) from comment #5) > > The remaining errors are: > > > > HelperAppDialog.js > > 213:13 error "ContentAreaUtils" is not defined no-undef > > > > Is this inherited from browser.js? > > Yeah, looks like this was probably copy/pasted from browser.js, where we do > define this: > > http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/ > browser.js#374 > > But looking how HelperAppDialog.js is loaded, I don't think it loads in the > global window context, so this is probably busted... we should file another > bug about this to look into whether or not this logic works. Probably we > just need to add this lazy getter definition. > > > PromptService.js > > 399:60 error "aText" is not defined no-undef > > 401:49 error "aText" is not defined no-undef > > > > I imagine aText is supposed to be in the method header, but I can't find the > > idl definition. > > Looks like text should be a parameter here... > http://mxr.mozilla.org/mozilla-central/source/netwerk/base/nsIAuthPrompt. > idl#31 > > wesj, is there just a typo in here that we're missing an aText parameter? > http://mxr.mozilla.org/mozilla-central/source/mobile/android/components/ > PromptService.js#384 > > > Snippets.js > > 256:24 error Unexpected identifier > > > > I don't think it likes yield, but HelperAppDialog didn't have any issues. > > > > Any ideas, Margaret? > > Maybe because gStatsPath is a global defined in a lazy getter? > http://mxr.mozilla.org/mozilla-central/source/mobile/android/components/ > Snippets.js#59
> Maybe because gStatsPath is a global defined in a lazy getter? I added gStatsPath as a global but it didn't work. There is no rule included here so it's likely a parse error. I wonder if there is another option we need to enable in order for it to recognize this feature.
(In reply to Michael Comella (:mcomella) from comment #2) > > Maybe because gStatsPath is a global defined in a lazy getter? > > I added gStatsPath as a global but it didn't work. There is no rule included > here so it's likely a parse error. I wonder if there is another option we > need to enable in order for it to recognize this feature. I spoke with :Standard8 and it seems a generator function should be specified as `function* ()`, so the parser was not expecting to see `yield` in a `function ()`.
(In reply to Michael Comella (:mcomella) from comment #3) > (In reply to Michael Comella (:mcomella) from comment #2) > > > Maybe because gStatsPath is a global defined in a lazy getter? > > > > I added gStatsPath as a global but it didn't work. There is no rule included > > here so it's likely a parse error. I wonder if there is another option we > > need to enable in order for it to recognize this feature. > > I spoke with :Standard8 and it seems a generator function should be > specified as `function* ()`, so the parser was not expecting to see `yield` > in a `function ()`. Ah, yeah, this code is old, and was written before we standardized on that generator function syntax. That may be a common problem in our code :/
Assignee: nobody → michael.l.comella
Bug 1177774 - Explicitly declare generator functions. r=margaret Use `function* () { ... }`.
Attachment #8626733 - Flags: review?(margaret.leibovic)
Bug 1177774 - Ignore non-standard "catch ex if" in lint. r=margaret
Attachment #8626734 - Flags: review?(margaret.leibovic)
Bug 1177774 - Ignore non-standard array comprehension in lint. r=margaret
Attachment #8626735 - Flags: review?(margaret.leibovic)
Bug 1177774 - Fix warnings in HomeProvider. r=margaret
Attachment #8626737 - Flags: review?(margaret.leibovic)
Bug 1177774 - Add global variable to hide lint error. r=margaret
Attachment #8626739 - Flags: review?(margaret.leibovic)
Bug 1177774 - Disable "onmessage =" warning. r=margaret Not sure what to do here so I filed bug 1177901.
Attachment #8626740 - Flags: review?(margaret.leibovic)
Bug 1177774 - Disable warnings for "aText". r=margaret
Attachment #8626741 - Flags: review?(margaret.leibovic)
Comment on attachment 8626736 [details] MozReview Request: Bug 1177774 - Update SharedPreferencesImpl constructor arg. r=nalexander r+ via irl.
Attachment #8626736 - Flags: review?(nalexander) → review+
Comment on attachment 8626733 [details] MozReview Request: Bug 1177774 - Explicitly declare generator functions. r=margaret https://reviewboard.mozilla.org/r/12117/#review10589 Ship It!
Attachment #8626733 - Flags: review?(margaret.leibovic) → review+
Comment on attachment 8626734 [details] MozReview Request: Bug 1177774 - Ignore non-standard "catch ex if" in lint. r=margaret https://reviewboard.mozilla.org/r/12119/#review10591 Ship It!
Attachment #8626734 - Flags: review?(margaret.leibovic) → review+
Comment on attachment 8626735 [details] MozReview Request: Bug 1177774 - Ignore non-standard array comprehension in lint. r=margaret https://reviewboard.mozilla.org/r/12121/#review10593 Ship It!
Attachment #8626735 - Flags: review?(margaret.leibovic) → review+
Comment on attachment 8626736 [details] MozReview Request: Bug 1177774 - Update SharedPreferencesImpl constructor arg. r=nalexander https://reviewboard.mozilla.org/r/12123/#review10595 Ship It!
Attachment #8626736 - Flags: review+
Comment on attachment 8626737 [details] MozReview Request: Bug 1177774 - Import ContentAreaUtils. r=margaret https://reviewboard.mozilla.org/r/12125/#review10599 ::: mobile/android/modules/HomeProvider.jsm:122 (Diff revision 1) > - let success = HomeProvider.requestSync(datasetId, callback); > + let success = this.HomeProvider.requestSync(datasetId, callback); I'm not sure this actually works... wouldn't `this` refer to the `syncTimerCallback` context? I think you'll need to decalre a global variable for the global context, or add some rule to ignore this error. ::: mobile/android/modules/HomeProvider.jsm:142 (Diff revision 1) > - ValidationError: ValidationError, > + ValidationError: this.ValidationError, Same question here... wouldn't `this` here refer to the `HomeProvider` object?
Attachment #8626737 - Flags: review?(margaret.leibovic)
Comment on attachment 8626736 [details] MozReview Request: Bug 1177774 - Update SharedPreferencesImpl constructor arg. r=nalexander https://reviewboard.mozilla.org/r/12123/#review10601 Ship It!
Attachment #8626736 - Flags: review+
Comment on attachment 8626738 [details] MozReview Request: Bug 1177774 - Add global variable to hide lint error. r=margaret https://reviewboard.mozilla.org/r/12127/#review10603 Ship It!
Attachment #8626738 - Flags: review?(margaret.leibovic) → review+
Comment on attachment 8626739 [details] MozReview Request: Bug 1177774 - Disable "onmessage =" warning. r=margaret https://reviewboard.mozilla.org/r/12129/#review10605 Ship It!
Attachment #8626739 - Flags: review?(margaret.leibovic) → review+
Comment on attachment 8626740 [details] MozReview Request: Bug 1177774 - Disable warnings for "aText". r=margaret https://reviewboard.mozilla.org/r/12131/#review10607 Ship It!
Attachment #8626740 - Flags: review?(margaret.leibovic) → review+
Comment on attachment 8626741 [details] MozReview Request: Bug 1177774 - Fix issues in HomeProvider. r=margaret https://reviewboard.mozilla.org/r/12133/#review10609 Ship It!
Attachment #8626741 - Flags: review?(margaret.leibovic) → review+
Comment on attachment 8626737 [details] MozReview Request: Bug 1177774 - Import ContentAreaUtils. r=margaret Bug 1177774 - Import ContentAreaUtils. r=margaret
Attachment #8626737 - Attachment description: MozReview Request: Bug 1177774 - Fix warnings in HomeProvider. r=margaret → MozReview Request: Bug 1177774 - Import ContentAreaUtils. r=margaret
Attachment #8626737 - Flags: review?(margaret.leibovic)
Comment on attachment 8626738 [details] MozReview Request: Bug 1177774 - Add global variable to hide lint error. r=margaret Bug 1177774 - Add global variable to hide lint error. r=margaret
Attachment #8626738 - Attachment description: MozReview Request: Bug 1177774 - Import ContentAreaUtils. r=margaret → MozReview Request: Bug 1177774 - Add global variable to hide lint error. r=margaret
Attachment #8626738 - Flags: review+ → review?(margaret.leibovic)
Comment on attachment 8626739 [details] MozReview Request: Bug 1177774 - Disable "onmessage =" warning. r=margaret Bug 1177774 - Disable "onmessage =" warning. r=margaret Not sure what to do here so I filed bug 1177901.
Attachment #8626739 - Attachment description: MozReview Request: Bug 1177774 - Add global variable to hide lint error. r=margaret → MozReview Request: Bug 1177774 - Disable "onmessage =" warning. r=margaret
Attachment #8626739 - Flags: review+ → review?(margaret.leibovic)
Attachment #8626740 - Attachment description: MozReview Request: Bug 1177774 - Disable "onmessage =" warning. r=margaret → MozReview Request: Bug 1177774 - Disable warnings for "aText". r=margaret
Attachment #8626740 - Flags: review+ → review?(margaret.leibovic)
Comment on attachment 8626740 [details] MozReview Request: Bug 1177774 - Disable warnings for "aText". r=margaret Bug 1177774 - Disable warnings for "aText". r=margaret
Attachment #8626741 - Attachment description: MozReview Request: Bug 1177774 - Disable warnings for "aText". r=margaret → MozReview Request: Bug 1177774 - Fix issues in HomeProvider. r=margaret
Attachment #8626741 - Flags: review+ → review?(margaret.leibovic)
Comment on attachment 8626741 [details] MozReview Request: Bug 1177774 - Fix issues in HomeProvider. r=margaret Bug 1177774 - Fix issues in HomeProvider. r=margaret
https://reviewboard.mozilla.org/r/12125/#review10613 > Same question here... wouldn't `this` here refer to the `HomeProvider` object? Deleted this commit and added a new one.
Attachment #8626737 - Flags: review?(margaret.leibovic) → review+
Comment on attachment 8626737 [details] MozReview Request: Bug 1177774 - Import ContentAreaUtils. r=margaret https://reviewboard.mozilla.org/r/12125/#review10615 Ship It!
Attachment #8626741 - Flags: review?(margaret.leibovic) → review+
Comment on attachment 8626741 [details] MozReview Request: Bug 1177774 - Fix issues in HomeProvider. r=margaret https://reviewboard.mozilla.org/r/12133/#review10617 Ship It!
Attachment #8626738 - Flags: review?(margaret.leibovic) → review+
Attachment #8626739 - Flags: review?(margaret.leibovic) → review+
Attachment #8626740 - Flags: review?(margaret.leibovic) → review+
I backed out the HomeProvider bit for causing bug 1178739. https://hg.mozilla.org/integration/fx-team/rev/504678ee14db https://hg.mozilla.org/releases/mozilla-aurora/rev/00c94def1c35 I'll morph that bug to be about addressing the HomeProvider lint errors.
Depends on: 1178739
Depends on: 1178703
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: