Repository navigation
Conversation
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Hypothesis LMS assignments built from Canvas Pages show the page through Via. By
default Via rewrites every iframe in a page so it also loads through Via.
Canvas Pages ask Via not to do that with
via.proxy_frames=0(hypothesis/lms#6045, Feb 2024), so embedded players load directly from their
own sites.
That flag was only honoured server-side, in the HTML viahtml rewrites.
wombat.js, pywb's client-side rewriter, kept proxying any iframe a page script
inserted or re-inserted after load. Page-builder runtimes such as DesignPlus
rebuild the page that way, so on those pages a public-link Canvas Studio embed
ended up proxied anyway, and Via's JavaScript rewriting breaks Studio's player
(same failure mode as Kanopy in product-backlog#1760).
This is one of the two causes behind product-backlog#1748 and #1749. The other,
toolbar (plug-icon) embeds that use Canvas's
external_tools/retrieveroute,cannot work inside a frame at all and is handled in hypothesis/lms#7479 by
turning them into a link.
Changes
Honour
via.proxy_frames=0in client-side rewriting too. Aproxy_framestemplate helper reads the flag the same way the server-side hook does. When it
is off,
head_insert.htmlwraps wombat'srewriteUrlso that iframe and frameURLs (modifiers
if_andfr_) are returned unchanged. Every iframe-rewritingpath in wombat goes through that method, so this covers iframes added via
innerHTML, the
srcproperty,setAttributeandinsertAdjacentHTML. Allother resources are rewritten as before, and pages without the flag are
untouched.
Guard the override against pywb changes. The wrapper only installs if
_WBWombat.prototype.rewriteUrlis a function. If a future pywb renames it,behaviour falls back to today's instead of throwing on the first rewrite.
Testing
Unit test for the helper (flag "0" and junk disable, "1" and absent keep the
default), and a functional test that the override is in the served page only
when the flag is "0".
Verified locally against a page that re-inserts its iframes after load, served
through local viahtml with the flag:
/proxy/if_/...andthe Studio player broke
Not covered: the server-side flag being ignored under load (shared
hooks.contextwith gevent), which is a separate change.