Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: improved experience on DNSLink websites #826

Merged
merged 3 commits into from
Dec 6, 2019
Merged

feat: improved experience on DNSLink websites #826

merged 3 commits into from
Dec 6, 2019

Conversation

lidel
Copy link
Member

@lidel lidel commented Dec 5, 2019 •
edited

This PR changes default behavior and disables redirect of DNSLink websites
to local gateway, as noted in #701 (comment)

Context and motivation

TODO

  • disable DNSLink redirect by default
  • display IPFS actions on DNSLink websites (without redirect)
  • add DNSLink section to Preferences screen
    • enabling DNSLink redirect is off by default, requires explicit opt-in.
    • If user enables it, we warn about breaking of Origin sandbox:

      2019-12-05--23-22-39

  • ensure IPFS recovery works if DNSLink's HTTP server is down
  • update browser action menu to display item for opening DNSLink website at Custom Gateway (in a new tab) (963a340)
    • remove per-domain redirect toggle from main menu (c7c3221)
  • preload of visited DNSLink URLs to local node is added to ensure content is still distributed and is available if original HTTP server is down (see feat: preload visited DNSLink URLs to local node #827)

Closes #701, #667, helps with ipfs-inactive/docs#271
cc @cwaring @hugomrdias @autonome

@lidel
feat: opt-in DNSLink redirect
24869d5 
In past, it was opt-out via redirect opt-out per hostname.
This changes default behavior and disables redirect of DNSLink websites
to local gateway, as noted in
#701

Adds DNSLink section to Preferences screen.

Closes #701, #667
autonome
autonome reviewed Dec 6, 2019
View reviewed changes
add-on/_locales/en/messages.json Outdated
@@ -275,6 +275,18 @@
"message": "Redirect requests for IPFS resources to the Custom gateway",
"description": "An option description on the Preferences screen (option_useCustomGateway_description)"
},
"option_dnslinkRedirect_title": {
"message": "Force load from Custom Gateway",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe change to: "Force page load from custom gateway"

  1. Explicit language specifying object that is being loaded
  2. Consistent capitalization

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably fine as is, but I am a little on the fence about "force".

"Always load pages from custom gateway"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with the language tweak here. This might be out of scope for this PR but do you think it's widely understood what a Custom Gateway is? I would be inclined to be more specific, such as: Always load DNSLink websites via IPFS Companion's local gateway. (if that is a correct statement?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will tweak the label, but want to keep "force" for now to emphasize its "over the top" behavior (until we can redirect in a way that keeps origin sandbox)

@cwaring ack, lets track that as part of #741

@cwaring cwaring mentioned this pull request Dec 6, 2019
Closed
@lidel lidel mentioned this pull request Dec 6, 2019
Closed
1 task
@lidel
feat: View on Gateway context action on IPFS pages
963a340 
This adds new item to Active Tab section of menu in browser action
that enables user to open existing IPFS resource at Public or Custom
Gateway (depends on redirect status).
@lidel
feat: remove 'redirect on {domain}' from main menu
c7c3221 
This removes "Redirect on {fqdn}" menu item from browser action menu.
DNSLink redirect is no longer enabled by default, and this feature does
not need to be present at all times (It remains on Preferences screen if
needed).

In the future we will change it to "Disaple IPFS integrations on these
hostnames"

Part of #741
@lidel
Copy link
Member Author

lidel commented Dec 6, 2019

Simplified main menu and added option to View on Gateway which opens current tab via Gateway in a new tab:

view-on-gateway2019-12-06--20-02-43

Due to #808 my plan is to fast-track this to Beta (and then Stable in a day or two), but feel free to provide feedback here or in new issue, we can improve UI in new PR.

@lidel lidel marked this pull request as ready for review December 6, 2019 19:35
@lidel lidel requested a review from cwaring December 6, 2019 19:36
@lidel lidel changed the title feat: opt-in DNSLink redirect feat: improved experience on DNSLink websites Dec 6, 2019
@lidel lidel merged commit df96a05 into master Dec 6, 2019
@lidel lidel deleted the fix/stop-breaking-dnslink-origin branch December 6, 2019 19:38
@momack2
Copy link

momack2 commented Dec 6, 2019

Needs review and explicit approval by @jbenet

lidel added a commit that referenced this pull request Dec 9, 2019
@lidel
feat: preload DNSLink websites in background
124d299 
With
#826
data was no longer getting into the cache of local node.
This adds background queue that ensures every visited DNSLink asset
is preloaded to the cache of local IPFS node.
lidel added a commit that referenced this pull request Dec 9, 2019
@lidel
feat: preload DNSLink websites in background
2b7c145 
With
#826
data was no longer getting into the cache of local node.
This adds background queue that ensures every visited DNSLink asset
is preloaded to the cache of local IPFS node.
@lidel lidel mentioned this pull request Dec 9, 2019
Merged
lidel added a commit that referenced this pull request Dec 9, 2019
@lidel
feat: preload DNSLink websites in background
9344b6f 
With
#826
data was no longer getting into the cache of local node.
This adds background queue that ensures every visited DNSLink asset
is preloaded to the cache of local IPFS node.
@lidel
Copy link
Member Author

lidel commented Dec 9, 2019 •
edited

The second leg of this change is in #827, where preload of visited DNSLink URLs to local node is added to ensure content is still distributed and is available if original HTTP server is down.

Update: we restored redirect by default in #831

lidel added a commit that referenced this pull request Dec 10, 2019
@lidel
feat: preload visited DNSLink URLs to local node (#827)
6c37c6a 
With #826
data was no longer getting into the cache of a local node 
if DNSLink redirect is disabled by the user.

This adds background queue that ensures every visited DNSLink
asset is preloaded to the cache of a local IPFS node, 
if the user chooses to do so.
@lidel lidel mentioned this pull request Dec 10, 2019
Merged
@github-actions github-actions bot mentioned this pull request Feb 16, 2023
Closed
@github-actions github-actions bot mentioned this pull request Sep 21, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@autonome autonome autonome left review comments

@cwaring cwaring Awaiting requested review from cwaring

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Disable default redirect of DNSLink websites if it breaks Origin
4 participants
@lidel @momack2 @autonome @cwaring