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

Add notifications to imports #15176

Closed
spylogsster opened this issue Apr 7, 2021 · 5 comments · Fixed by brave/brave-core#8422
Closed

Add notifications to imports #15176

spylogsster opened this issue Apr 7, 2021 · 5 comments · Fixed by brave/brave-core#8422

Comments

@spylogsster
Copy link

spylogsster commented Apr 7, 2021

Description

Created from @bbondy comment brave/brave-core#8422 (comment)

Right clicking on a YouTube video (you need to right click twice because youtube tries to override the context menu), allows you to select import Video. But then it gives an error.

IPFS Companion also gives an error.
The error seems OK, but the way we deal with the error is that we don't do anything. IPFS companion does a notification like this for the error. We could do something similar unless you have a better suggestion.

image

Actual result:

  • Nothing happens

Expected result:

  • Show notification about import error

Reproduces how often:

easy

Brave version (brave://version info)

Version/Channel Information:

  • Can you reproduce this issue with the current release?
  • Can you reproduce this issue with the beta channel?
  • Can you reproduce this issue with the nightly channel?

Other Additional Information:

  • Does the issue resolve itself when disabling Brave Shields?
  • Does the issue resolve itself when disabling Brave Rewards?
  • Is the issue reproducible on the latest version of Chrome?

Miscellaneous Information:

@lidel
Copy link

lidel commented Apr 8, 2021

I wrote some details about import failing on some websites in ipfs/ipfs-companion#227 (mostly Firefox, but I feel a variant applies here too).

TLDR: In IPFS Companion it fails due to tracking protection (either regular CORS and other headers set by website, or browser itself behaving more strict for some reason, like in Firefox with protection enabled).

We can't do much in a browser extension, but Brave should be able to side-steps those protections if the request was triggered by user interaction with native Import UI.

@spylogsster spylogsster changed the title Add notifications about import errors Add notifications to imports Apr 8, 2021
@spylogsster
Copy link
Author

image

@bbondy bbondy added this to the 1.25.x - Nightly milestone Apr 16, 2021
@stephendonner stephendonner added the QA/In-Progress Indicates that QA is currently in progress for that particular issue label Apr 20, 2021
@stephendonner
Copy link

stephendonner commented Apr 20, 2021

Verified PASSED using the testplan from brave/brave-core#8422 with build

Brave 1.25.28 Chromium: 90.0.4430.72 (Official Build) nightly (x86_64)
Revision b6172ef8d07ef486489a4b11b66b2eaeed50d132-refs/branch-heads/4430@{#1233}
OS macOS Version 11.2.3 (Build 20D91)

Steps:

(YouTube video) - import failure

  1. loaded https://www.youtube.com/watch?v=_28sorQWw_E&t=22s
  2. right-clicked on the video (twice)
  3. chose Import to IPFS > Selected video
  4. got the import-error message notification below

(MP4 video) - import success

  1. loaded https://www.learningcontainer.com/mp4-sample-video-files-download/
  2. right-clicked on the 1st video
  3. chose Import to IPFS > Selected video
  4. got the IPFS Import Completed message notification below
  5. confirmed I was able to share the video via link: https://bafybeics24zwxekzwol2brpsubz6es6awjn2eolvpa3jlafqgt4qxvbm3q.ipfs.dweb.link/?filename=sample-mp4-file.mp4

(HTML page) - import partly complete

  1. loaded http://en.wikipedia-on-ipfs.org.ipns.localhost:48081/wiki/
  2. right-clicked on the page
  3. chose Import to IPFS > This page
  4. got the import-success message
  5. again, right-clicked on the page
  6. chose Import to IPFS > This page
  7. confirmed I got the IPFS import partly completed message notification
example example example
Screen Shot 2021-04-19 at 6 30 17 PM Screen Shot 2021-04-19 at 6 28 35 PM Screen Shot 2021-04-20 at 2 54 54 PM

@stephendonner
Copy link

@spylogsster @lidel do either of you happen to have a handy use-case for IPFS import partly completed? Thanks!

@spylogsster
Copy link
Author

spylogsster commented Apr 20, 2021

@stephendonner try to import same link twice, next imports will be partially completed because same link already exists in target directory

@stephendonner stephendonner added QA Pass-macOS and removed QA/In-Progress Indicates that QA is currently in progress for that particular issue labels Apr 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment