Firefox WebExtensions misdetected as Chrome Packaged Apps

[robertknight]

I'm submitting a ...

• bug report
• feature request
• other (Please do not submit support requests here (see above))

Current behavior:

In the current version of Firefox WebExtensions, window.chrome and window.chrome.runtime exist but window.chrome.app does not. A check in $sniffer misdetects the application as a Chrome Packaged App. This in turn disables the HTML 5 History API support in the $location service causing unexpected behaviour elsewhere. In the case of our app it caused the $location service to try and redirect the browser to a hash-bangified version (/path/app.html => #!/path/app.html) of the URL on startup, breaking the app.

Expected / new behavior:

The isChromePackagedApp check referenced above should return false in an HTML page loaded from a Firefox WebExtension.

Angular version: 1.5.11
Browser: [Firefox 51]

Other information

A workaround for Firefox WebExtension authors is to set window.chrome.app to a dummy object before Angular bootstraps so that the check does not produce a false positive.

[frederikprijck]

Hi @robertknight,

As you tested it with AngularJS 1.5.11, could you verify the issue also occurs with 1.6.4 ?
A change was made regarding the sniffer, which was released in 1.6.2 (see: 7019d9f ).

I doubt that change is solving your problem, but it's always interesting to know whether or not the behavior is different.

[robertknight]

Thank-you for the quick response. The isChromePackagedApp expression from the master branch of this repo still incorrectly evaluates to a truthy value in Firefox, including in 1.6.4, so the issue still applies.

[frederikprijck]

Unless you want to provide a PR, I wouldn't mind creating one myself.

Any idea how we should be validating this correctly ?
/cc @gkalpak @Narretz

[Narretz]

If window.chrome.app isn't true in a FF web extension, how can the check in the sniffer for packaged apps be true? It specifically checks for window.chrome.app

edit: I didn't see the second window.chrome.app check is reversed with !

[frederikprijck]

If I get this straight, the following is happening:

isChromePackagedApp =
            !isNw &&
            $window.chrome &&
            ($window.chrome.app && $window.chrome.app.runtime ||
                !$window.chrome.app && $window.chrome.runtime && $window.chrome.runtime.id),

Which evaluates to

isChromePackagedApp =
            true &&
            true &&
            (false && true ||
                true && true && true),

Which results in true.

The change @robertknight made in the referenced PR in his repo (see https://github.com/hypothesis/client/pull/460/files) changes this so that window.chrome.app is overridden and no runtime property is available on it which will evaluate to:

isChromePackagedApp =
            true &&
            true &&
            (true && false ||
                false && false&& false),

Which will evaluate to false and fixes the issue for him.

[robertknight]

If I get this straight, the following is happening:

You've got it right.

If you're able to create a PR that would be great. A question I have is whether it is necessary to explicitly test for Chrome packaged apps / NW.js instead of feature-detecting the availability of the history APIs. According to this comment, window.history.{pushState, replaceState} are not defined in Chrome Apps. I'm not certain if that is the case & always has been.

[frederikprijck]

Good question, this is the second time an issue like this arises (5 months ago the one for NW apps was fixed).

Maybe @gkalpak or @Narretz have an idea on why it's being checked this way instead of feature detection.

I guess it has to do with:

// Chrome Packaged Apps are not allowed to access `history.pushState`.
// If not sandboxed, they can be detected by the presence of `chrome.app.runtime`
// (see https://developer.chrome.com/apps/api_index). If sandboxed, they can be detected by
// the presence of an extension runtime ID and the absence of other Chrome runtime APIs
// (see https://developer.chrome.com/apps/manifest/sandbox).
// (NW.js apps have access to Chrome APIs, but do support `history`.)

But my knowlegde of that is limited (if existing).

[Narretz]

IIRC the problem is that Chrome Packaged Apps will do something bad when you try to access history.pushState.

[gkalpak]

Right. You can find more details in #13945.