fix: remove normalizeLinksContentScript #836
Merged
Conversation
This file contains 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
The script was responsible for overriding all src and href in DOM of every page to ensure content addressed with ipfs:// URL loads. Looking back, it does not align well with our upgrade path, and the result across browser vendors was buggy. While it allowed loading simple images, it did not help with broken XHR and iframe quirks and introduced unnecessary battery drain. Due to this, it is better to remove it and advise users to use content-addressing at a public gateway. This way everything works for people without IPFS client, and browsers with IPFS support can easily detect it and upgrade the call.
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.
This PR removes
normalizeLinkscontent script, which run by default. IPFS Companion will no longer process the DOM of every page, which should benefit overall browsing performance, and make it easier to test and reason about protocol handler support across browser vendors.Motivation
The script executed on every page. Was responsible for overriding all
srcandhrefattributes found in DOM with value fromwindow.ipfsCompanionPubGwURL.The goal was to ensure content addressed with ipfs:// URL "just loads", but in reality it worked only for simple
<img>and<a>tags.Looking back, it does not align well with our opportunistic-upgrade-path, and the result across browser vendors was buggy at best. While it allowed loading simple images, it did not help with broken XHR, iframe quirks (PR closes #816) and introduced unnecessary battery drain. The test page at https://ipfs.github.io/in-web-browsers/ipfs-protocol-handler-support-tests.html is a good demonstration of how partial solution it was.
Due to this, it is better to remove it and advise users to use content-addressing at a public gateway. That way upgrade path is more robust: everything works for people without IPFS client, and browsers with IPFS support can easily detect it and upgrade the call.
See my suggestion posted to original issue: #286 (comment)