-
-
Notifications
You must be signed in to change notification settings - Fork 238
Add inject_scripts option #967
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
base: main
Are you sure you want to change the base?
Conversation
This option enables injecting custom scripts from static/ into replayed pages in both the client-side and server-side replay modes. This is useful for emulating removed browser features (such as emulating Flash Player with Ruffle) or applying compatibility or behavior tweaks.
tw4l
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the submission! I suggested a few in-line changes, and have one question around whether we should also support injecting scripts from the per-collection static directories.
| archiveMod: "ir_", | ||
| adblockUrl: this.adblockUrl, | ||
| noPostToGet: true, | ||
| injectScripts: this.injectScripts.map(src => "../" + src), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pywb supports static files from the root static directory or per-collection static directories: https://pywb.readthedocs.io/en/latest/manual/ui-guide.html#static-files. It might be worth applying that same logic to the injected scripts, in case users want e.g. Ruffle enabled but only on one collection?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this is exactly what you meant, but I've updated it so you can do this:
inject_scripts:
- all.js
- other.js
collections:
mycoll:
inject_scripts:
- all.js # static/all.js
- _/mycoll/tweaks.js # collections/mycoll/static/tweaks.jsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh and the injectScripts.map(src => "../" + src) on this line is because wabac.js actually adds the prefix /static/proxy/. It looked to me like maybe the original idea was for it to load absolute URLs from the live web, but the serviceworker just returned 404 when I tried that, but ../ worked.
ldko
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works for me..."injected script" is loading in server or client side replay. I hit an error on one of the two sites I was testing with: Uncaught TypeError: can't access property "wb_info", this is undefined when running with client_side_replay: false and enabling ruffle via inject_scripts. However, I was able to reproduce this issue for that archived site on the main branch when enabling ruffle via head_insert.html, so this is not a new problem. I just had a couple comments on the docs. Thanks @ato!
Co-authored-by: Lauren Ko <[email protected]>
Co-authored-by: Lauren Ko <[email protected]>
|
Thanks for the changes! |
|
@ato Seeing new test failures related to last commit (noticed after approving as last check before merging) - e.g.: |
|
The test failure could be triggered if the collection is one of the special ones: |
Co-authored-by: Lauren Ko <[email protected]>
|
Oh, my bad, thanks for catching that Tessa. And thanks for for the fix Lauren.
Yeah, I guess you can at least use the top-level setting for them and in a pinch you could work around it by having the injected JavaScript file itself check the collection name. To allow for per-collection options though it would probably make sense if '$live' and '$all' were just shorthand for something like: collections:
all:
type: all
live:
type: livebut that's probably better tackled separately to this.
It seems ok? At least I don't notice anything obviously wrong when I setup the directories under collections:
test1:
inject_scripts:
- test1.js
test2:
inject_scripts:
- test2.js |
Description
This adds an option to inject custom scripts from static/ into replayed pages in both the client-side and server-side replay modes.
Motivation and Context
This is useful for emulating removed browser features, and applying compatibility or behavior tweaks. For example:
document.layers = trueon a specific site to bypass "You must use Netscape 4.x"We've been injecting scripts by overriding
head_insert.html(for server-side replay mode) andloadWabac.js(for client-side replay mode) but it would be cleaner to have a single option that works for both modes. It would also be nice to not have to repatch the templates each time we update pywb.Types of changes
Checklist: