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)
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
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(wjohnston)
Assignee | ||
Comment 1•9 years ago
|
||
To fix the links above:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#374
http://mxr.mozilla.org/mozilla-central/source/netwerk/base/nsIAuthPrompt.idl#31
http://mxr.mozilla.org/mozilla-central/source/mobile/android/components/PromptService.js#384
http://mxr.mozilla.org/mozilla-central/source/mobile/android/components/Snippets.js#59
Assignee | ||
Comment 2•9 years ago
|
||
> 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.
Assignee | ||
Comment 3•9 years ago
|
||
(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 ()`.
Comment 4•9 years ago
|
||
(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 | ||
Updated•9 years ago
|
Blocks: android-jslint
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → michael.l.comella
Assignee | ||
Comment 6•9 years ago
|
||
Bug 1177774 - Explicitly declare generator functions. r=margaret
Use `function* () { ... }`.
Attachment #8626733 -
Flags: review?(margaret.leibovic)
Assignee | ||
Comment 7•9 years ago
|
||
Bug 1177774 - Ignore non-standard "catch ex if" in lint. r=margaret
Attachment #8626734 -
Flags: review?(margaret.leibovic)
Assignee | ||
Comment 8•9 years ago
|
||
Bug 1177774 - Ignore non-standard array comprehension in lint. r=margaret
Attachment #8626735 -
Flags: review?(margaret.leibovic)
Assignee | ||
Comment 9•9 years ago
|
||
Bug 1177774 - Update SharedPreferencesImpl constructor arg. r=nalexander
Attachment #8626736 -
Flags: review?(nalexander)
Assignee | ||
Comment 10•9 years ago
|
||
Bug 1177774 - Fix warnings in HomeProvider. r=margaret
Attachment #8626737 -
Flags: review?(margaret.leibovic)
Assignee | ||
Comment 11•9 years ago
|
||
Bug 1177774 - Import ContentAreaUtils. r=margaret
Attachment #8626738 -
Flags: review?(margaret.leibovic)
Assignee | ||
Comment 12•9 years ago
|
||
Bug 1177774 - Add global variable to hide lint error. r=margaret
Attachment #8626739 -
Flags: review?(margaret.leibovic)
Assignee | ||
Comment 13•9 years ago
|
||
Bug 1177774 - Disable "onmessage =" warning. r=margaret
Not sure what to do here so I filed bug 1177901.
Attachment #8626740 -
Flags: review?(margaret.leibovic)
Assignee | ||
Comment 14•9 years ago
|
||
Bug 1177774 - Disable warnings for "aText". r=margaret
Attachment #8626741 -
Flags: review?(margaret.leibovic)
Assignee | ||
Comment 15•9 years ago
|
||
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 16•9 years ago
|
||
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 17•9 years ago
|
||
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 18•9 years ago
|
||
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 19•9 years ago
|
||
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 20•9 years ago
|
||
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 21•9 years ago
|
||
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 22•9 years ago
|
||
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 23•9 years ago
|
||
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 24•9 years ago
|
||
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 25•9 years ago
|
||
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+
Assignee | ||
Comment 26•9 years ago
|
||
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)
Assignee | ||
Comment 27•9 years ago
|
||
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)
Assignee | ||
Comment 28•9 years ago
|
||
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)
Assignee | ||
Updated•9 years ago
|
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)
Assignee | ||
Comment 29•9 years ago
|
||
Comment on attachment 8626740 [details]
MozReview Request: Bug 1177774 - Disable warnings for "aText". r=margaret
Bug 1177774 - Disable warnings for "aText". r=margaret
Assignee | ||
Updated•9 years ago
|
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)
Assignee | ||
Comment 30•9 years ago
|
||
Comment on attachment 8626741 [details]
MozReview Request: Bug 1177774 - Fix issues in HomeProvider. r=margaret
Bug 1177774 - Fix issues in HomeProvider. r=margaret
Assignee | ||
Comment 31•9 years ago
|
||
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.
Updated•9 years ago
|
Attachment #8626737 -
Flags: review?(margaret.leibovic) → review+
Comment 32•9 years ago
|
||
Comment on attachment 8626737 [details]
MozReview Request: Bug 1177774 - Import ContentAreaUtils. r=margaret
https://reviewboard.mozilla.org/r/12125/#review10615
Ship It!
Updated•9 years ago
|
Attachment #8626741 -
Flags: review?(margaret.leibovic) → review+
Comment 33•9 years ago
|
||
Comment on attachment 8626741 [details]
MozReview Request: Bug 1177774 - Fix issues in HomeProvider. r=margaret
https://reviewboard.mozilla.org/r/12133/#review10617
Ship It!
Updated•9 years ago
|
Attachment #8626738 -
Flags: review?(margaret.leibovic) → review+
Updated•9 years ago
|
Attachment #8626739 -
Flags: review?(margaret.leibovic) → review+
Updated•9 years ago
|
Attachment #8626740 -
Flags: review?(margaret.leibovic) → review+
Assignee | ||
Comment 34•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/c084008baf54
https://hg.mozilla.org/integration/fx-team/rev/6392ad38e5f0
https://hg.mozilla.org/integration/fx-team/rev/8da7c13207e2
https://hg.mozilla.org/integration/fx-team/rev/86d872344c2d
https://hg.mozilla.org/integration/fx-team/rev/8ffc52a3af14
https://hg.mozilla.org/integration/fx-team/rev/8e40cbea086d
https://hg.mozilla.org/integration/fx-team/rev/5f54a4e0b36b
https://hg.mozilla.org/integration/fx-team/rev/ae0144e27599
https://hg.mozilla.org/integration/fx-team/rev/8cab4b5abe7f
Comment 35•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c084008baf54
https://hg.mozilla.org/mozilla-central/rev/6392ad38e5f0
https://hg.mozilla.org/mozilla-central/rev/8da7c13207e2
https://hg.mozilla.org/mozilla-central/rev/86d872344c2d
https://hg.mozilla.org/mozilla-central/rev/8ffc52a3af14
https://hg.mozilla.org/mozilla-central/rev/8e40cbea086d
https://hg.mozilla.org/mozilla-central/rev/5f54a4e0b36b
https://hg.mozilla.org/mozilla-central/rev/ae0144e27599
https://hg.mozilla.org/mozilla-central/rev/8cab4b5abe7f
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Comment 36•9 years ago
|
||
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.
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•