Closed Bug 1320782 Opened 3 years ago Closed 3 years ago

Numerous warnings about possible CPOW usage in browser_ext_omnibox.js

Categories

(WebExtensions :: Frontend, defect)

defect
Not set

Tracking

(firefox53 fixed)

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: jaws, Assigned: mattw)

References

Details

Attachments

(1 file)

From running ./mach eslint browser/components/extensions

> msunu@DESKTOP-I07UH4B /c/cedar
> $ mach eslint browser/components/extensions/
> c:\cedar\browser\components\extensions\test\browser\browser_ext_omnibox.js
>   197:32  warning  content is a possible Cross Process Object Wrapper (CPOW).  mozilla/no-cpows-in-tests (eslint)
>   197:32  warning  content is a possible Cross Process Object Wrapper (CPOW).  mozilla/no-cpows-in-tests (eslint)
>   204:57  warning  content is a possible Cross Process Object Wrapper (CPOW).  mozilla/no-cpows-in-tests (eslint)
>   205:64  warning  content is a possible Cross Process Object Wrapper (CPOW).  mozilla/no-cpows-in-tests (eslint)
>   233:6   warning  content is a possible Cross Process Object Wrapper (CPOW).  mozilla/no-cpows-in-tests (eslint)
>   234:6   warning  content is a possible Cross Process Object Wrapper (CPOW).  mozilla/no-cpows-in-tests (eslint)
>   235:6   warning  content is a possible Cross Process Object Wrapper (CPOW).  mozilla/no-cpows-in-tests (eslint)
>   242:57  warning  content is a possible Cross Process Object Wrapper (CPOW).  mozilla/no-cpows-in-tests (eslint)
>   243:63  warning  content is a possible Cross Process Object Wrapper (CPOW).  mozilla/no-cpows-in-tests (eslint)
>   244:63  warning  content is a possible Cross Process Object Wrapper (CPOW).  mozilla/no-cpows-in-tests (eslint)
> 
> Γ£û 10 problems (0 errors, 10 warnings)
I don't see the value of this warning since all it does is check the name of the variable. For the omnibox API, `content` is the established name used to describe the string that is put in the URL bar - https://developer.chrome.com/extensions/omnibox#type-SuggestResult. Do you think it makes sense to fix each of these warnings or should I just disable this warning for the test?
Flags: needinfo?(jaws)
I'm okay with disabling the warning for the test.
Flags: needinfo?(jaws)
Comment on attachment 8815439 [details]
Bug 1320782 - Disable eslint no-cpows-in-tests check for browser_ext_omnibox.js

https://reviewboard.mozilla.org/r/96340/#review96560

::: browser/components/extensions/test/browser/browser_ext_omnibox.js:6
(Diff revision 1)
>  /* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
>  /* vim: set sts=2 sw=2 et tw=80: */
>  "use strict";
>  
> +// The no-cpows-in-tests check isn't very smart, simply warning if it finds
> +// a variable named `content`. The Omnibox API requires that name for setting the

This comment seems extreme, it "requires" that name?  Or it just happens to use it?
Attachment #8815439 - Flags: review?(aswan) → review+
Comment on attachment 8815439 [details]
Bug 1320782 - Disable eslint no-cpows-in-tests check for browser_ext_omnibox.js

https://reviewboard.mozilla.org/r/96340/#review96560

Thanks

> This comment seems extreme, it "requires" that name?  Or it just happens to use it?

It just uses it for Chrome compatibility, so I updated it to say that.
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/a2e634f475d3
Disable eslint no-cpows-in-tests check for browser_ext_omnibox.js r=aswan
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/a2e634f475d3
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Assignee: nobody → mwein
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.