Closed Bug 1280027 Opened 3 years ago Closed Last year

Reject manifests with any "incognito" property other than "spanning"

Categories

(WebExtensions :: General, defect, P3)

defect

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: bsilverberg, Unassigned)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [manifest]triaged)

This is a follow-up to bug 1277686 to change from a warning to an error when a manifest contains an `incognito` property other than `spanning`.
Assignee: nobody → bob.silverberg
Status: NEW → ASSIGNED
Whiteboard: [manifest]triaged
Kris, you suggested we land this as a warning first, and then make it a hard error. At what point do you think we should land this error version? Bug 1277686 has landed in all versions going back to 48.
Flags: needinfo?(kmaglione+bmo)
Please make sure to add a way to specify different values for Chrome and Firefox in the same manifest, before making it an error.

For example, you could add support for this:

{
  "applications": {
    "gecko": {
      "incognito": "spanning"
    }
  },
  "incognito": "split"
}

My add-on's manifest contains incognito:split, because the add-on cannot work in Google Chrome in their incognito mode without this value. If I set the value to incognito:spanning, then Chrome will refuse to load any tabs containing my extension pages in incognito windows, and will instead show an error message containing the code ERR_BLOCKED_BY_CLIENT. My add-on code is written to work in both modes, and Firefox does not have the same limitation as Chrome where add-on tabs cannot be opened in incognito windows when using spanning mode.
Has this landed to AMO yet? We are getting this error here - https://github.com/Noitidart/Chrome-Store-Foxified/issues/47
No, it will still reject manifests that do not say "spanning" for incognito mode, as the validation results show. I currently know of no bug on file to loosen that restriction on AMO.
I was pointed here when I filed https://github.com/mozilla/addons/issues/254
Bob, I think we'd be good to land this sometime in 53.
Component: WebExtensions: Untriaged → WebExtensions: General
Flags: needinfo?(kmaglione+bmo)
Priority: -- → P3
The meaning of split incognito mode, and the lack thereof in Firefox should be documented.

(In reply to Jesper Kristensen from comment #2)
> Please make sure to add a way to specify different values for Chrome and
> Firefox in the same manifest, before making it an error.
> 
> For example, you could add support for this:
> 
> {
>   "applications": {
>     "gecko": {
>       "incognito": "spanning"
>     }
>   },
>   "incognito": "split"
> }

This sounds like a good idea. I think, something more generic like the following would be more useful.

"conditional": [{
    "condition": {
        "min_firefox_version": 45
    },
    "actions": [{
        "type": "set",       // type can be "delete" (and "append" for arrays)
        "path": "incognito", // path can also be something like content_scripts[0].matches
        "value": "spanning"
    }]
}

(if this is considered a good idea I don't mind implementing it; create a bug and assign it to me)
Keywords: dev-doc-needed
At present, the "incognito" flag is totally useless in Firefox. The default behavior, with no "incognito" flag, is
that content script WebExtensions are loaded and run for incognito pages.  (This is "spanning" behavior, the only
allowed explicit value per the above.) That's not a good default, and it is a big change from Jetpack behavior,
where, by default, add-ons did not run on incognito pages. 

Documentation does not indicate this default: 

https://developer.mozilla.org/en-US/Add-ons/WebExtensions/manifest.json/incognito

At least support "not allowed" as a valid option. 

This should either be fixed, or the feature clearly documented as useless in Firefox, along with a security
warning that WebExtensions run in incognito mode by default.
Blocks: 1460738
Product: Toolkit → WebExtensions
Assignee: bob.silverberg → nobody
Status: ASSIGNED → NEW
This already gives an error loading on values other than spanning.  Since we're working towards supporting the other values, am wontfixing this.
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.