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.

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

IPFS Protocol Indicator in Location Bar #398

Closed
lidel opened this issue Feb 23, 2018 · 3 comments · Fixed by #418
Closed

IPFS Protocol Indicator in Location Bar #398

lidel opened this issue Feb 23, 2018 · 3 comments · Fixed by #418
Assignees
Labels
exp/novice Someone with a little familiarity can pick up good first issue Good issue for new contributors help wanted Seeking public contribution on this issue status/in-progress In progress
Milestone

Comments

@lidel
Copy link
Member

lidel commented Feb 23, 2018

Background

WebExtensions API pageAction can be used for adding an icon to the location bar:
page-action

Chrome does not allow extensions to register both browserAction and pageAction (that is why we have context-actions such as pin/unpin in browserAction).

But Firefox does not have such limitation 👌

Feature Description

Here's an idea for additional UX on non-Chrome browsers:

  • we could use pageAction for indicating loaded URL is an IPFS resource
    • aqua (online) icon indicates custom external or embedded node was used for loading content
    • grey (offline) icon indicates public gateway was used
    • icon is hidden for non-IPFS URLs

Notes on Interop

  • it may be necessary to create build:chrome target that removes page_action from manifest.json
  • isChrome needs to be added to runtime-checks.js and code responsible for showing/hiding pageAction needs to be wrapped with !isChrome

References

@lidel lidel added help wanted Seeking public contribution on this issue UX good first issue Good issue for new contributors exp/novice Someone with a little familiarity can pick up labels Feb 23, 2018
@lidel lidel added this to the 2018-Q1 milestone Feb 23, 2018
@cvan
Copy link

cvan commented Mar 7, 2018

@lidel: This is a good issue to address. Good catch on the pageAction and browserAction discrepancies between Chrome/Opera and Firefox/Edge. (This isn't documented in nor addressed in the code base of Mozilla's web-extension-polyfill.)

I see in this repo that there's already a script for transforming the manifest for Jenkins CI.

I've seen other "universal" WebExtension projects have separate npm scripts for each browser build target (usually involving gulp too, which I would not recommend adding as a dependency to this project). Though I'd prefer that Chrome fixes its behaviour to be consistent with WebExtension's, do you agree that the best option is to add build: and watch: scripts to transform the manifest.json for each build target? (I can file a Chromium bug, but do you know if one is already filed re: this behaviour?)

Admittedly, this all adds quite a bit of complexity. But an optimal end-user experience is undeniably more important. I'll think about this some more; if you have any other ideas, I'm all ears. I'm happy to help here.

P.S. Great work (to everyone involved here) on keeping up with the WebExtensions, pushing IPFS in the browser space, and filing these great, comprehensive issues.

@lidel
Copy link
Member Author

lidel commented Mar 7, 2018

@cvan Thank you for kind words!

Historically we were doing our best to have single build artefact, but these interop discrepancies add up over time, and just like you noted we will have separate build pipelines for different targets quite soon. There is an opt-in manifest-related workaround for our self-hosted Beta for Firefox (e9f77f7), but it is just a PoC. We will move it from bash & jq to npm script and add other targets. The only way to go forward, it seems.

I did not see Chrome bug or any discussion about simultaneous use of pageAction and browserAction in manifest. It might be that people see this error message and assume it is a conscious design decision by the Chrome team.

@lidel lidel added the status/ready Ready to be worked label Mar 7, 2018
lidel added a commit that referenced this issue Mar 8, 2018
As noted in #398
Firefox supports additional WebExtension APIs, but Chrome throws
an error on them.

This change splits manifest into common and vendor-specific files
and creates a dedicated bundle for each.
@lidel lidel self-assigned this Mar 12, 2018
@lidel lidel added status/in-progress In progress and removed status/ready Ready to be worked labels Mar 12, 2018
lidel added a commit that referenced this issue Mar 12, 2018
@lidel
Copy link
Member Author

lidel commented Mar 12, 2018

Check initial implementation in PR #418.
Feedback about UX (icon, tooltip) would be appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exp/novice Someone with a little familiarity can pick up good first issue Good issue for new contributors help wanted Seeking public contribution on this issue status/in-progress In progress
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants