Closed Bug 1241993 Opened 4 years ago Closed 4 years ago

Fix eslint errors in nsBrowserGlue.js

Categories

(Firefox :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 46
Tracking Status
firefox46 --- fixed

People

(Reporter: jaws, Assigned: jaws)

References

Details

Attachments

(1 file)

No description provided.
I'm not sure why but nsBrowserGlue.js was still being ignored due to the eslintignore file, but I thought removing that line would be enough.

jared@jaws-win10 /c/fx
$ mach eslint browser/components/nsBrowserGlue.js
 0:00.97 Running c:\Users\jared\AppData\Roaming\npm\eslint
 0:00.97 c:/mozilla-build/msys/bin/sh.exe -c c:/Users/jared/AppData/Roaming/npm/eslint --plugin html --ext [.js,.jsm,.jsx,.xml,.html] browser/components/nsBrowserGlue.js

c:\fx\browser\components\nsBrowserGlue.js
  0:0  warning  File ignored because of your .eslintignore file. Use --no-ignore to override

✖ 1 problem (0 errors, 1 warning)

 0:02.14 Finished eslint. No errors encountered.

jared@jaws-win10 /c/fx
$ mach eslint browser/components/nsBrowserGlue.js --no-ignore
 0:00.96 Running c:\Users\jared\AppData\Roaming\npm\eslint
 0:00.96 c:/mozilla-build/msys/bin/sh.exe -c c:/Users/jared/AppData/Roaming/npm/eslint --plugin html --ext [.js,.jsm,.jsx,.xml,.html] browser/components/nsBrowserGlue.js --no-ignore
 0:02.10 Finished eslint. No errors encountered.
Comment on attachment 8711131 [details]
MozReview Request: Bug 1241993 - Fix eslint errors in nsBrowserGlue.js. r?gijs

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/31995/diff/1-2/
Attachment #8711131 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8711131 [details]
MozReview Request: Bug 1241993 - Fix eslint errors in nsBrowserGlue.js. r?gijs

https://reviewboard.mozilla.org/r/31995/#review28835

I intend to land this with the fixes noted here, either in a bit or early next morning if nobody else does. :-)

::: browser/components/nsBrowserGlue.js:2969
(Diff revision 2)
> -#ifdef E10S_TESTING_ONLY
> +if (AppConstants.E10S_TESTING_ONLY) {

This doesn't actually seem useful, we only call into this when this is true anyway, and because the var will be hoisted, the variable will be included in the window, too. Add-ons don't seem to use it. Just always declare it.

::: toolkit/modules/AppConstants.jsm:250
(Diff revision 2)
> +  MOZ_REQUIRE_SIGNING:

You also need to move:

https://dxr.mozilla.org/mozilla-central/rev/d6d81655dd9e146c300a64c0fcaeb04ca3300a19/browser/components/moz.build#63-64

to the moz.build file in this directory.
https://hg.mozilla.org/mozilla-central/rev/c6e584cf706f
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
[bugday-20160323]

Status: RESOLVED,FIXED -> UNVERIFIED

Comments:
STR: Not clear.
Developer specific testing

Component: 
Name			Firefox
Version			46.0b9
Build ID		20160322075646
Update Channel          beta
User Agent		Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS			Windows 7 SP1 x86_64

Expected Results: 
Developer specific testing

Actual Results: 
As expected
Depends on: 1303380
You need to log in before you can comment on or make changes to this bug.