Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

HTML Video failing to load in Safari #478

Closed
mattpilott opened this issue Oct 17, 2018 · 6 comments
Closed

HTML Video failing to load in Safari #478

mattpilott opened this issue Oct 17, 2018 · 6 comments

Comments

@mattpilott
Copy link

Hello,

I'm running into an issue where html video is failing to play seemingly due to the service worker caching. My use case is that i'm pulling in html from a wordpress api and in the received html is a video. That video fails to play on safari more often than not. Seems to work in Chrome a lot more reliably.

After some trouble shooting between @pngwn and myself via discord @pngwn found that commenting out the last block of code enables the video to play:

event.respondWith(
  caches
  .open(`offline${timestamp}`)
  .then(async cache => {
    try {
      const response = await fetch(event.request);
      cache.put(event.request, response.clone());
      return response;
    } catch (err) {
      const response = await cache.match(event.request);
      if (response) return response;

      throw err;
    }
  })
);

Not wanting to lose this caching feature I thought it best to flag it here and link to the test repo for further inspection.

https://github.com/matt3224/vidtest.git

@Conduitry
Copy link
Member

No Safari here to check, but: In service-worker.js at the top of the fetch handler, can you try also adding a condition to bail if event.request.headers.has('range')? So:

if (event.request.method !== 'GET' || event.request.headers.has('range')) return;

Hopefully this will make the service worker not try to cache (or serve from cache) any of the range requests that safari makes.

@mattpilott
Copy link
Author

@pngwn suggested if (event.request.url.includes(".mp4")) return; but this seems more rounded as a solution, I have added, tested and can confirm that this seems to work. Thank you so much! Do you think this should/could be added to the default sapper service-worker.js ?

@Conduitry
Copy link
Member

Yeah probably. Even without the weird Safari behavior, there's probably just about only confusion to be gained by cache range requests. Having that as the default sounds good. Everything in sapper-template is a user-serviceable part anyway, so people can easily change that if they want different behavior.

@Conduitry
Copy link
Member

Updated the official template as I described above, and closing this issue. Thanks!

@mattpilott
Copy link
Author

@Conduitry look what I just came across, could be helpful perhaps?

https://philna.sh/blog/2018/10/23/service-workers-beware-safaris-range-request/

@Conduitry
Copy link
Member

Oh neat, I guess it is possible to get service workers to respond with partial content! I haven't looked at the implementation too closely, but that might be a nice-to-have for Sapper, apart from the Safari video thing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants