harvard-lil / perma

Indelible links
420 stars 71 forks source link

Use WACZ-exhibitor for playback in Perma (behind a feature flag) #3353

Closed rebeccacremona closed 1 year ago

rebeccacremona commented 1 year ago

See ENG-99.

wacz-exhibitor was built to help websites embed WARC and WACZ replays in the best way possible: we’d do well to use it in Perma!

Our proposed move to WACZ would allow us to fully benefit from its caching smarts, but we will benefit a bit even while Perma is still primarily serving WARC playbacks.

This PR

This PR adds the new Django setting PLAYBACK_HOST, set to None by default.

If defined, the application will send playbacks to that host instead of to the replay Django app currently served at a subdomain.

Locally,

to direct playbacks to an nginx container serving a slightly tweaked version of wacz-exhibitor.

You can toggle back and forth without restarting the Django dev server.

Most playbacks should look identical.

Exceptions

Interstitials

While working to re-implement the custom interstitials that handle any playbacks that should trigger downloads... I discovered that they are currently broken 😢: Safari users do not receive a usable download, and other users receive multiple copies. (Bug to be written up shortly.)

So, this PR does NOT re-implement the more complex interstitials we are currently using in prod, which were introduced specifically for Safari.

In production, when a user clicks the "View/Download File" button, we pry into the replay-web-page instance and find the internal URL for the payload. Then, we insert an <a> into the DOM with that as href and click it. That used to reliably trigger a download in all browsers... but alas: in Safari, that network request is NOT intercepted by the service worker, and falls through to the live web, where it either 404s (replay.perma.cc) or 500s (wacz-exhibitor).

I cannot think of any further workarounds... So, I just do the simpler thing, our original strategy: don't stick the playback iframe into the page until the user clicks the "View/Download File" button. That reliably triggers one download in all browsers tested except for Safari.

We probably want to talk about this some more... but not as part of this PR.

Fallback for old browsers

In production, replay.perma.cc is responsible for rendering an "update your browser!" message, if a person with a very old browser loads a Perma Link.

When playbacks are mediated by wacz-exhibitor, the perma app takes care of it. (TBD: whether that's something that ought to be handled by wacz-exhibitor instead; I think probably, but could be wrong!)

I didn't use an identical strategy: this code will attempt a playback for every browser that can read script type=module. That might not be good enough 🤷‍♀️ .... but I honestly don't know. Close enough for jazz? If not, we can readdress in a future PR.

This PR arranges for users to see the "update your browser!" message even when playback is mediated by an interstitial; with replay.perma.cc playbacks, the message is hidden behind the interstitial.

Deploying

I think this is safe to merge as-is, it should be a no-op without a setting of the feature flag, except for some trivial changes to interstitial CSS.

codecov[bot] commented 1 year ago

Codecov Report

Merging #3353 (e3c7101) into develop (b9d2089) will decrease coverage by 0.13%. The diff coverage is 50.00%.

@@             Coverage Diff             @@
##           develop    #3353      +/-   ##
===========================================
- Coverage    70.04%   69.92%   -0.13%     
===========================================
  Files           55       55              
  Lines         7054     7059       +5     
===========================================
- Hits          4941     4936       -5     
- Misses        2113     2123      +10     
Impacted Files Coverage Δ
perma_web/perma/models.py 86.19% <33.33%> (-0.14%) :arrow_down:
perma_web/perma/views/common.py 82.31% <66.66%> (-0.24%) :arrow_down:

... and 1 file with indirect coverage changes

rebeccacremona commented 1 year ago

@bensteinberg can you help me find this changed character in this test?

bensteinberg commented 1 year ago

Lines 188 and 196, I think

matteocargnelutti commented 1 year ago

👏 👏 👏 🚀