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

Status: RESOLVED WONTFIX

(WebExtensions :: General, defect, P3)

Description

[Bob Silverberg [:bsilverberg]]

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`.

Comment 1

[Bob Silverberg [:bsilverberg]]

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.

Comment 2

[Jesper Kristensen]

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.

Comment 3

[noitidart]

Has this landed to AMO yet? We are getting this error here - https://github.com/Noitidart/Chrome-Store-Foxified/issues/47

Comment 4

[Andy McKay]

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.

Comment 5

[Jesper Kristensen]

I was pointed here when I filed https://github.com/mozilla/addons/issues/254

Comment 6

[Andy McKay]

Bob, I think we'd be good to land this sometime in 53.

Comment 7

[Rob Wu [:robwu]]

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)

Comment 8

[John Nagle]

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.

Comment 9

[Shane Caraveo (:mixedpuppy)]

This already gives an error loading on values other than spanning.  Since we're working towards supporting the other values, am wontfixing this.