Skip to content
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

Error with isStreamingResponses in Safari #122

Closed
aristath opened this issue Jan 21, 2019 · 3 comments · Fixed by #128
Closed

Error with isStreamingResponses in Safari #122

aristath opened this issue Jan 21, 2019 · 3 comments · Fixed by #128
Milestone

Comments

@aristath
Copy link
Contributor

I've had reports from clients that on Safari they can't access a website where PWA was enabled. The error they were getting is shown in the screenshot below:
image823

From what i can tell isStreamingResponses only exists here:
https://github.com/xwp/pwa-wp/blob/733563f46996ea2d5aaf52a576971f92c3748164/wp-includes/js/service-worker-navigation-routing.js#L6-L21

Getting this fixed was a bit urgent for me, so commenting out these lines that enqueued the script seemed to get the job done
https://github.com/xwp/pwa-wp/blob/733563f46996ea2d5aaf52a576971f92c3748164/wp-includes/components/class-wp-service-worker-navigation-routing-component.php#L475-L481

@westonruter
Copy link
Collaborator

I recall now from @miina that Safari fails to properly handle const variables that are defined lexically in this way. If you switch to let or var does that make the problem go away?

@aristath
Copy link
Contributor Author

I currently don't have Safari to test, but @ilicfilip tested changing these 3 lines from const to var and that fixed the issue: https://github.com/xwp/pwa-wp/blob/dcdc5795f8d5f90b008a5db034ccb5357a9f5c11/wp-includes/js/service-worker-navigation-routing.js#L6-L8

aristath added a commit to aristath/pwa-wp that referenced this issue Jan 28, 2019
aristath added a commit to aristath/pwa-wp that referenced this issue Jan 29, 2019
aristath added a commit to aristath/pwa-wp that referenced this issue Jan 29, 2019
@aristath
Copy link
Contributor Author

@westonruter pushed 👍

westonruter added a commit that referenced this issue Feb 1, 2019
…ta.1

* 'master' of github.com:xwp/pwa-wp:
  tweak check for $manifest['icons']
  phpcs & phpunit fix
  PHP 5.2 compatibility & remove superfluous condition. See #118
  Another one for #118
  Add extra meta for iOS
  Another one for #122
  IIFE for #122
  fixes #122
  Update wp-includes/service-workers.php
  Fix code style
  Fix code style
  Fix front/admin url detection when port specified
  Add theme-color theme-support
  move filter to the get_theme_color method
  see #116
westonruter added a commit that referenced this issue Feb 4, 2019
…s-flag

* 'master' of github.com:xwp/pwa-wp: (24 commits)
  Remove unused global console
  Include missing base_url when precaching scripts & styles
  tweak check for $manifest['icons']
  phpcs & phpunit fix
  PHP 5.2 compatibility & remove superfluous condition. See #118
  Upgrade Workbox to 4.0.0-beta.2
  Upgrade workbox-cli to 4.0.0-beta.2
  Another one for #118
  Add extra meta for iOS
  Another one for #122
  IIFE for #122
  fixes #122
  Update wp-includes/service-workers.php
  Ensure legacy caching strategy factory names convert to class names
  Replace factory invocation of strategies with class instantiation
  Update skipWaiting and clientsClaim to use core namespace
  Upgrade Workbox to v4.0.0-beta.1
  Bump workbox-cli to 4.0.0-beta.1
  Fix code style
  Fix code style
  ...
@westonruter westonruter added this to the 0.2 milestone Apr 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants