New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

history.pushState is not available in packaged apps. #11932

Closed
rpgobjects opened this Issue May 22, 2015 · 16 comments

Comments

Projects
None yet
8 participants
@rpgobjects

rpgobjects commented May 22, 2015

I get this error message when I run my chrome apps since m43 of chrome. Seems related to this change. https://codereview.chromium.org/988293002

It doesn't have an negative effect on my apps so far, but I only get it when I use angular (3.15) in my chrome apps.

I saw a related closed issue.
#1358

I'm using the angular-csp.css file in my app.

@jdiamond

This comment has been minimized.

Show comment
Hide comment
@jdiamond

jdiamond May 24, 2015

This prevents the error message from appearing for me:

angular
    .module('app', [], function($provide) {
        // Prevent Angular from sniffing for the history API
        // since it's not supported in packaged apps.
        $provide.decorator('$window', function($delegate) {
            $delegate.history = null;
            return $delegate;
        });
    })
;

jdiamond commented May 24, 2015

This prevents the error message from appearing for me:

angular
    .module('app', [], function($provide) {
        // Prevent Angular from sniffing for the history API
        // since it's not supported in packaged apps.
        $provide.decorator('$window', function($delegate) {
            $delegate.history = null;
            return $delegate;
        });
    })
;
@caitp

This comment has been minimized.

Show comment
Hide comment
@caitp

caitp May 24, 2015

Contributor

well judging from the history of this, it seems like this feature has been disabled in chrome platform apps for a very long time (the bug is about getting rid of an extended IDL attribute and making the bindings layer simpler --- was this working for you during some window in packaged apps before M43?)

At any rate, @jdiamond's decorator trick will work for now, so maybe we can close this? I don't think we have a reliable way of detecting whether we're running in crippled API mode or not

Contributor

caitp commented May 24, 2015

well judging from the history of this, it seems like this feature has been disabled in chrome platform apps for a very long time (the bug is about getting rid of an extended IDL attribute and making the bindings layer simpler --- was this working for you during some window in packaged apps before M43?)

At any rate, @jdiamond's decorator trick will work for now, so maybe we can close this? I don't think we have a reliable way of detecting whether we're running in crippled API mode or not

@Narretz Narretz added this to the Purgatory milestone Jun 11, 2015

@raghavcreddy

This comment has been minimized.

Show comment
Hide comment
@raghavcreddy

raghavcreddy Feb 2, 2016

The decorator is not working anymore starting from Chrome 48.0.2564.97 (64-bit).

Is there any workaround?

raghavcreddy commented Feb 2, 2016

The decorator is not working anymore starting from Chrome 48.0.2564.97 (64-bit).

Is there any workaround?

@gkalpak

This comment has been minimized.

Show comment
Hide comment
@gkalpak

gkalpak Feb 2, 2016

Member

It is strange, because this workaround shouldn't be affected by browser behavior.
Do you have a (preferrably minimal) working demo of the issue ?

Member

gkalpak commented Feb 2, 2016

It is strange, because this workaround shouldn't be affected by browser behavior.
Do you have a (preferrably minimal) working demo of the issue ?

@raghavcreddy

This comment has been minimized.

Show comment
Hide comment
@raghavcreddy

raghavcreddy Feb 3, 2016

Here's a minimal working demo of the issue. It's a Chrome packaged app.
https://github.com/raghavcreddy/ChromeAppAngular

raghavcreddy commented Feb 3, 2016

Here's a minimal working demo of the issue. It's a Chrome packaged app.
https://github.com/raghavcreddy/ChromeAppAngular

@gkalpak

This comment has been minimized.

Show comment
Hide comment
@gkalpak

gkalpak Feb 3, 2016

Member

Thx @raghavcreddy. I'll take a look.

Member

gkalpak commented Feb 3, 2016

Thx @raghavcreddy. I'll take a look.

@gkalpak

This comment has been minimized.

Show comment
Hide comment
@gkalpak

gkalpak Feb 3, 2016

Member

@raghavcreddy, indeed window.history isn't writable in Chrome 48.
I tried defining a new history getter on window and it seemed to work (see below). It achieves the same effect as the original workaround.

.config(function($provide) {
  $provide.decorator('$window', function($delegate) {
    Object.defineProperty($delegate, 'history', {get: () => null});
    return $delegate;
  });
});

// Or using the `.decorator()` helper:
.decorator('$window', function($delegate) {
  Object.defineProperty($delegate, 'history', {get: () => null});
  return $delegate;
});
Member

gkalpak commented Feb 3, 2016

@raghavcreddy, indeed window.history isn't writable in Chrome 48.
I tried defining a new history getter on window and it seemed to work (see below). It achieves the same effect as the original workaround.

.config(function($provide) {
  $provide.decorator('$window', function($delegate) {
    Object.defineProperty($delegate, 'history', {get: () => null});
    return $delegate;
  });
});

// Or using the `.decorator()` helper:
.decorator('$window', function($delegate) {
  Object.defineProperty($delegate, 'history', {get: () => null});
  return $delegate;
});
@raghavcreddy

This comment has been minimized.

Show comment
Hide comment
@raghavcreddy

raghavcreddy commented Feb 4, 2016

Thank you @gkalpak

gkalpak added a commit to gkalpak/angular.js that referenced this issue Feb 4, 2016

fix($sniffer): fix history sniffing in Chrome Packaged Apps
Although `window.history` is present in the context of Chrome Packaged Apps, it is not allowed to
access `window.history.pushState` or `window.history.state`, resulting in errors when trying to
"sniff" history support.
This commit fixes it by detecting a Chrome Packaged App (through the presence of
`window.chrome.app.runtime`). Note that `window.chrome.app` is present in the context of "normal"
webpages as well, but it doesn't have the `runtime` property, which is only available to packaged
apps (e.g. see https://developer.chrome.com/apps/api_index).

Fixes angular#11932
@gkalpak

This comment has been minimized.

Show comment
Hide comment
@gkalpak

gkalpak Feb 4, 2016

Member

I have a fix for this is #13945. If that gets merged, then the workaround won't be necessary.
@raghavcreddy, btw, I think in Chrome Packaged Apps you should be running in CSP mode.

Member

gkalpak commented Feb 4, 2016

I have a fix for this is #13945. If that gets merged, then the workaround won't be necessary.
@raghavcreddy, btw, I think in Chrome Packaged Apps you should be running in CSP mode.

@schellmax

This comment has been minimized.

Show comment
Hide comment
@schellmax

schellmax Feb 4, 2016

thanks a lot for sharing the workaround.
for those not familiar with ES6 yet (like me), here's the 'back-ported' version ;)

    .decorator('$window', function($delegate) {
        Object.defineProperty($delegate, 'history', {get: function(){ return null; }});
        return $delegate;
    })

schellmax commented Feb 4, 2016

thanks a lot for sharing the workaround.
for those not familiar with ES6 yet (like me), here's the 'back-ported' version ;)

    .decorator('$window', function($delegate) {
        Object.defineProperty($delegate, 'history', {get: function(){ return null; }});
        return $delegate;
    })
@gkalpak

This comment has been minimized.

Show comment
Hide comment
@gkalpak

gkalpak Feb 4, 2016

Member

BTW, if someone could try a (preferrably non-trivial Chrome Packaged App) with the files from this build and let me know if it logs any errors and if everything works as expected, that would awesome !
(Make sure NOT to include the workaround.)

Member

gkalpak commented Feb 4, 2016

BTW, if someone could try a (preferrably non-trivial Chrome Packaged App) with the files from this build and let me know if it logs any errors and if everything works as expected, that would awesome !
(Make sure NOT to include the workaround.)

@schellmax

This comment has been minimized.

Show comment
Hide comment
@schellmax

schellmax Feb 6, 2016

@gkalpak i've removed the workaround in my project and replaced angular.js by the one from your build, works just fine, no more errors in chrome app

schellmax commented Feb 6, 2016

@gkalpak i've removed the workaround in my project and replaced angular.js by the one from your build, works just fine, no more errors in chrome app

@gkalpak

This comment has been minimized.

Show comment
Hide comment
@gkalpak

gkalpak Feb 6, 2016

Member

@schellmax, that's good to know. Thx for trying it out 👍
All we need now is to get #13945 merged :)

Member

gkalpak commented Feb 6, 2016

@schellmax, that's good to know. Thx for trying it out 👍
All we need now is to get #13945 merged :)

@donkonstante

This comment has been minimized.

Show comment
Hide comment
@donkonstante

donkonstante Mar 21, 2016

@gkalpak I tried getting the files from that build but it's gone. I get error 404.

donkonstante commented Mar 21, 2016

@gkalpak I tried getting the files from that build but it's gone. I get error 404.

@gkalpak

This comment has been minimized.

Show comment
Hide comment
@gkalpak

gkalpak Mar 21, 2016

Member

@donkonstante, yeah orlder builds get deleted after some time. You can always checkout #13945 and build it yourself if you need it.

(I'll go lobby to get #13945 merged 😛)

Member

gkalpak commented Mar 21, 2016

@donkonstante, yeah orlder builds get deleted after some time. You can always checkout #13945 and build it yourself if you need it.

(I'll go lobby to get #13945 merged 😛)

@gkalpak gkalpak closed this in 9f5526f Mar 22, 2016

gkalpak added a commit that referenced this issue Mar 22, 2016

fix($sniffer): fix history sniffing in Chrome Packaged Apps
Although `window.history` is present in the context of Chrome Packaged Apps, it is not allowed to
access `window.history.pushState` or `window.history.state`, resulting in errors when trying to
"sniff" history support.
This commit fixes it by detecting a Chrome Packaged App (through the presence of
`window.chrome.app.runtime`). Note that `window.chrome.app` is present in the context of "normal"
webpages as well, but it doesn't have the `runtime` property, which is only available to packaged
apps (e.g. see https://developer.chrome.com/apps/api_index).

(It also also contains some style changes for making the structure and layout of `$sniffer` tests
 more consistent.)

Fixes #11932

Closes #13945
@gkalpak

This comment has been minimized.

Show comment
Hide comment
@gkalpak

gkalpak Mar 22, 2016

Member

It's in master and will be included in the upcoming 1.5.x release.
@donkonstante, until then, you can grab the latest code from https://code.angularjs.org/snapshot/.

Member

gkalpak commented Mar 22, 2016

It's in master and will be included in the upcoming 1.5.x release.
@donkonstante, until then, you can grab the latest code from https://code.angularjs.org/snapshot/.

Parent5446 added a commit to Parent5446/angular.js that referenced this issue Aug 12, 2016

fix($sniffer): don't use history.pushState for sandboxed Chrome apps
Check in $sniffer if operating in a sandboxed Chrome app, which does not
have access to chrome.app.runtime like other apps, but also does not
have access to history.pushState. If inside a sandboxed Chrome app, do
not try and use history.pushState, else an error will occur. See angular#11932
and angular#13945 for previous work.

gkalpak added a commit that referenced this issue Sep 8, 2016

fix($sniffer): don't use `history.pushState` in sandboxed Chrome Pack…
…aged Apps

While sandboxed Chrome Packaged Apps (CPAs) have the same restrictions wrt
accessing `history.pushState` as "normal" CPAs, they can't be detected in the
same way, as they do not have access to the same APIs.
Previously, due to their differences from normal CPAs, `$sniffer` would fail to
detect sandboxed CPAs and incorrectly assume `history.pushState` is available
(which resulted in an error being thrown).
This commit fixes the detection of sandboxed CPAs in `$sniffer`.

See #11932 and #13945 for previous work.

Closes #15021

gkalpak added a commit that referenced this issue Sep 8, 2016

fix($sniffer): don't use `history.pushState` in sandboxed Chrome Pack…
…aged Apps

While sandboxed Chrome Packaged Apps (CPAs) have the same restrictions wrt
accessing `history.pushState` as "normal" CPAs, they can't be detected in the
same way, as they do not have access to the same APIs.
Previously, due to their differences from normal CPAs, `$sniffer` would fail to
detect sandboxed CPAs and incorrectly assume `history.pushState` is available
(which resulted in an error being thrown).
This commit fixes the detection of sandboxed CPAs in `$sniffer`.

See #11932 and #13945 for previous work.

Closes #15021

petebacondarwin added a commit to petebacondarwin/angular.js that referenced this issue Nov 21, 2016

fix($sniffer): don't use `history.pushState` in sandboxed Chrome Pack…
…aged Apps

While sandboxed Chrome Packaged Apps (CPAs) have the same restrictions wrt
accessing `history.pushState` as "normal" CPAs, they can't be detected in the
same way, as they do not have access to the same APIs.
Previously, due to their differences from normal CPAs, `$sniffer` would fail to
detect sandboxed CPAs and incorrectly assume `history.pushState` is available
(which resulted in an error being thrown).
This commit fixes the detection of sandboxed CPAs in `$sniffer`.

See angular#11932 and angular#13945 for previous work.

Closes angular#15021

petebacondarwin added a commit to petebacondarwin/angular.js that referenced this issue Nov 21, 2016

fix($sniffer): don't use `history.pushState` in sandboxed Chrome Pack…
…aged Apps

While sandboxed Chrome Packaged Apps (CPAs) have the same restrictions wrt
accessing `history.pushState` as "normal" CPAs, they can't be detected in the
same way, as they do not have access to the same APIs.
Previously, due to their differences from normal CPAs, `$sniffer` would fail to
detect sandboxed CPAs and incorrectly assume `history.pushState` is available
(which resulted in an error being thrown).
This commit fixes the detection of sandboxed CPAs in `$sniffer`.

See angular#11932 and angular#13945 for previous work.

Closes angular#15021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment