Numerous warnings about possible CPOW usage in browser_ext_omnibox.js

RESOLVED FIXED in Firefox 53

Status

()

Toolkit
WebExtensions: Frontend
RESOLVED FIXED
a year ago
11 months ago

People

(Reporter: jaws, Assigned: mattw)

Tracking

unspecified
mozilla53
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

MozReview Requests

()

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

Attachments

(1 attachment)

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)
(Assignee)

Comment 1

a year ago
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 hidden (mozreview-request)

Comment 4

a year ago
mozreview-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

::: 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+
(Assignee)

Comment 5

a year ago
mozreview-review-reply
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.
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Keywords: checkin-needed

Comment 7

a year ago
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

Comment 8

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a2e634f475d3
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Assignee: nobody → mwein
You need to log in before you can comment on or make changes to this bug.