JS IPFS fallback #310

alanshaw · 2017-11-15T17:01:07Z

This PR adds js-ipfs to the extension and will fallback to using it if it cannot be connected within 20s.

There's still some work to do on the contextual menus, but the basics are there!

lidel

Thank you, looks exciting!

Some things to address before merging:

We should have a checkbox on the Preferences screen that disables this fallback
Browser action popup looks confusing. It kinda suggests that the user is using node at ipfs.io. Perhaps we should explicitly indicate that we are running in-browser JS-IPFS node for uploads with ipfs.io as public gateway?
Eg. add an item named "API" below "GATEWAY" and set it to "js-ipfs@127.0.0.1" when fallback is active?

lidel · 2017-11-15T23:51:48Z

add-on/src/lib/ipfs-companion.js

@@ -28,6 +30,40 @@ function initIpfsApi (ipfsApiUrl) {
  return window.IpfsApi({host: url.hostname, port: url.port, procotol: url.protocol})
 }

+async function fallbackToBrowserIpfs () {


Hm.. fallbackToLocalJsIpfs may be more clear.

lidel · 2017-11-15T23:52:29Z

add-on/src/lib/ipfs-companion.js

@@ -348,6 +384,10 @@ function handleMessageFromBrowserAction (message) {
  }
 }

+function isBrowserIpfs () {


How about isLocalJsIpfs ? It is less confusing.

alanshaw · 2017-11-16T12:49:08Z

We should have a checkbox on the Preferences screen that disables this fallback

I'm interested in your opinion on why that's necessary. I was thinking we might need it for when a user switches on their daemon after the fallback happens. I had originally considered that the extension should just automatically switch to using a running daemon if it became available...but I'm not convinced yet that that's a good idea!

I've just been talking to @olizilla and I think he's got a good point - right now, ipfs-companion is focused on technical users - you have to have a IPFS daemon running for the plugin to work. I think this is beyond many non-technical users, so actually it might be better to flip our perspective and make js-ipfs in the browser the default, and have a checkbox for "power users" to allow them to use their own daemon. I believe this will lower the barriers to entry and help adoption.

As a side thought - for your average non-technical user, waiting 20s to fallback to js-ipfs isn't a great experience, but for the expert, it gives them some time to fire up their terminal and start the daemon.

Either way, it's preferable to have an IPFS running than not, so i think falling back to js-ipfs if the user's configured daemon isn't running is still a good idea. If the user has selected the "use my own daemon" checkbox and js-ipfs is running then perhaps a button can be displayed to make the switch.

Browser action popup looks confusing. It kinda suggests that the user is using node at ipfs.io. Perhaps we should explicitly indicate that we are running in-browser JS-IPFS node for uploads with ipfs.io as public gateway?
Eg. add an item named "API" below "GATEWAY" and set it to "js-ipfs@127.0.0.1" when fallback is active?

I agree, it shouldn't suggest that the user is using the node at ipfs.io. Again, @olizilla pointed out that "GATEWAY" isn't useful information in the context of js-ipfs in the browser - it can't be changed, and has to be used, so we can just remove it.

I'm not keen on "API: js-ipfs@127.0.0.1" since js-ipfs in the browser doesn't expose an API, as it can't bind to an IP/port to do so. I think it's misleading on a number of fronts! I don't think we necessarily need to distinguish what type of IPFS node is being used. Non-technical users won't care/understand and technical users will likely know already.

Let me know that you think 😄 Awesome to get the conversation going! 🚀

olizilla · 2017-11-16T13:19:09Z

I'm basically just re-stating @lgierth and @lidel from this issue https://github.com/protocol/lab-week-unconf/issues/23 in that a goal is

Browser extension enables users to load IPFS content without any additional software

So if we view the add-on from the perspective of a new user, then you want it to just work. If we detect that a local ipfs daemon is running, then we're in pro-mode and we want to show all the info and options. I'm not saying that we hide everything from users without a local daemon, but it's interesting to think about what a user who is just trying out ipfs needs and how much info is appropriate.

lidel · 2017-11-16T23:28:42Z

(fantastic discussion!)

We should have a checkbox on the Preferences screen that disables [js-ipfs]
I'm interested in your opinion on why that's necessary.

There are many reasons why some users may want to opt-out from running local js-ipfs node, from the top of my head:

mobile users with bandwidth limits
impact on phone/laptop battery
memory leaks / stability issues (this is an uncharted territory, AFAIK nobody is running browserified js-ipfs for days without restart)
this is an experiment, and it is expected for every new experiment to have an off-switch 🌵

It might be better to flip our perspective and make js-ipfs in the browser the default, and have a checkbox for "power users" to allow them to use their own daemon. I believe this will lower the barriers to entry and help adoption.

I agree, changing the default will improve initial user experience.
As long as there will be a checkbox, I am okay with this change 👍

The checkbox can be named "Use my own daemon", "Use external API" or something like that:

when checked, js-ipfs is disabled, js-ipfs-api is used instead and "IPFS API URL" is visible and editable (current behaviour)
when unchecked, js-ipfs is used and "IPFS API URL" is hidden / replaced with static label "Running embedded js-ipfs node"
we probably want it unchecked by default (users that want to use own daemon need to opt-in)

So if we view the add-on from the perspective of a new user, then you want it to just work.

Yes, but we don't want to.. lie 🙃
I feel it is important to clarify some details related to our long term goal:

Browser extension enables users to load IPFS content without any additional software

The more specific, technical goal is to use js-ipfs for both reading and writing.

Writes to IPFS are possible today just fine (via ipfs.files.add), but reading (loading IPFS resources over IPFS) in WebExtension context is not possible yet, as noted in Problem #1: Inability to Inject HTTP Responses by WebExtension.

I am aware that this limitation may not exist in Brave, but we need to think about Firefox and Chrome users and try to keep experience similar across all browsers, where possible.

Note that when run in Firefox, js-ipfs fallback introduced in this PR routes all IPFS requests to a public HTTP gateway (default is https://ipfs.io/, but user can change it). And this is the scenario which always made me a bit uncomfortable, thinking there should be a better way to present this state to the user.

Right now browser action icon suggests user is "online", displays "swarm peers", it looks like we are connected to IPFS network:

While technically it is true, all content is loaded over public HTTP gateway. Not IPFS. When running js-ipfs there is no "enable redirect" button, nothing.

I want to avoid situation where this "partially working js-ipfs" feels disingenuous to users who expect extension to use distributed web for loading content.

Let me know if I am overthinking it 🙂
Perhaps "GATEWAY" label is good/clear enough.
The best I can come up with right now is to rename it to "HTTP GATEWAY", for clarity sake.
Not sure how to address this – any better ideas?

alanshaw · 2017-11-27T16:48:29Z

Hi @lidel,

We've had some thoughts since last week and we have a couple of changes and suggestions aimed to address some of the feedback above.

As we already discussed, and if you're still ok with the idea, then lets make JS-IPFS the default so first time (and non-technical) users get an awesome experience. However for this first pass lets park the fallback idea. I think it over complicates things and I'd rather just make it really easy to turn on your own IPFS node without having to start a daemon (i.e. start an embedded JS-IPFS node).

Since all the current ipfs-companion users are "power users" we should aim to make this as non-disruptive as possible by placing the toggle switch directly on the popup, so they can just switch back (and forth) really easily. I've done some mockups:

External

Embedded

We could also get the "stars" in the background of the header as per the website...you get the idea though :)

I think the embedded/external toggle deals with any ambiguity around what kind of IPFS node is being used, and with the logo/status being moved up to the top of the popup we have space to change "GATEWAY" to "HTTP GATEWAY" (not visualised).

@olizilla and I have the goal of getting IPFS into the Brave browser, and right now we're thinking we'll be able to feature detect what extension APIs are available and allow Brave users to have IPFS content served directly to them by JS-IPFS. If this becomes too complicated then we might have to fork, but we'd rather push the boundaries here in ipfs-companion and have all the browsers benefit from any improvements we make towards that goal.

I hope that maybe one day Firefox/Chrome browser extensions will allow IPFS to serve content. If that happens we'd no longer be being disingenuous towards Firefox/Chrome users and the gateway/redirect configurations go away*.

* ok so not completely, I see that there's definitely a use case for being able to use an external redirect that's not your local IPFS daemon.

I'd expect the toggle to also be on the settings page, so perhaps it's worth adding a message there where there's more real estate to explain the state of play with the embedded IPFS node and the extensions API?

lidel · 2017-11-27T21:58:22Z

@alanshaw I am sold. Wholeheartedly agree with proposed direction!

Let's do our best to keep single code base. Feel free to add necessary feature-detection and error handling to make it work in Brave. This is not a precedent, we already have some vendor-specific code, for example upload works bit differently under Firefox due to Right click upload via ipfs.util.addFromURL sometimes does not work #227:

ipfs-companion/add-on/src/lib/ipfs-companion.js

Line 241 in 7814ab1

if (inFirefox()) {

I suggest to create a similar utility function inBrave() so that Brave-specific sections are easy to spot and reason about. Also, if you find a better way of runtime detection than looking at the User-Agent, that would be neat (and more robust – current heuristic breaks when user changes User-Name).
Mockups are on point 👌 and lowkey address the GUI refresh (GUI refresh: share visual language used by other IPFS apps #273)
- The toggle is really nice and clean. Yes, it should have analogous, well-documented checkbox in the Preferences
  - I feel I could mitigate the disingenuous thing under non-Brave browsers by providing short description of Embedded option in a tool-tip when user hovers cursor on it. So do not worry about it :-)
- Now that we have more space on the bottom, we could explicitly list "API" address (under "HTTP GATEWAY"). I believe it would remove a lot of confusion for edge cases eg. when using local API but public HTTP gateway.
I was skeptical about defaulting to js-ipfs due to long-running process killing Firefox tabs in past, but now I don't think we need to worry about js-ipfs being ready, to push the work forward. Worst case scenario is that we just hide the toggle under Firefox/Chrome until it is stable enough to be enabled by default.

Keep up great work!

olizilla · 2017-11-28T16:46:24Z

Note: This PR has been superseded. The discussion has lead to #319 and #320, and there will be more.

I'm leaving it open for a little while longer to keep the conversation visible, and while I port things.

olizilla · 2017-12-12T14:47:35Z

See #320

alanshaw added 2 commits November 15, 2017 16:41

adds fallback to js-ipfs if cannot connect to IPFS daemon

78fa157

adds js-ipfs to test env

16fac56

lidel self-requested a review November 15, 2017 23:41

lidel requested changes Nov 15, 2017

View reviewed changes

lidel reviewed Nov 15, 2017

View reviewed changes

lidel mentioned this pull request Nov 16, 2017

Experiment: use js-ipfs as a fallback for uploading content when IPFS API is down #304

Closed

alanshaw mentioned this pull request Nov 28, 2017

adds popup header and adds browserify-css and tachyons #319

Merged

olizilla mentioned this pull request Dec 1, 2017

Init ipfs-api or js-ipfs #320

Merged

olizilla closed this Dec 12, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JS IPFS fallback #310

JS IPFS fallback #310

alanshaw commented Nov 15, 2017

lidel left a comment

lidel Nov 15, 2017

lidel Nov 15, 2017

alanshaw commented Nov 16, 2017

olizilla commented Nov 16, 2017

lidel commented Nov 16, 2017 •

edited

Loading

alanshaw commented Nov 27, 2017

lidel commented Nov 27, 2017 •

edited

Loading

olizilla commented Nov 28, 2017

olizilla commented Dec 12, 2017

JS IPFS fallback #310

JS IPFS fallback #310

Conversation

alanshaw commented Nov 15, 2017

lidel left a comment

Choose a reason for hiding this comment

lidel Nov 15, 2017

Choose a reason for hiding this comment

lidel Nov 15, 2017

Choose a reason for hiding this comment

alanshaw commented Nov 16, 2017

olizilla commented Nov 16, 2017

lidel commented Nov 16, 2017 • edited Loading

alanshaw commented Nov 27, 2017

External

Embedded

lidel commented Nov 27, 2017 • edited Loading

olizilla commented Nov 28, 2017

olizilla commented Dec 12, 2017

lidel commented Nov 16, 2017 •

edited

Loading

lidel commented Nov 27, 2017 •

edited

Loading