Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Saving a document doesn't show correct origin #8698

Closed
srirambv opened this issue May 4, 2017 · 9 comments
Closed

Saving a document doesn't show correct origin #8698

srirambv opened this issue May 4, 2017 · 9 comments

Comments

@srirambv
Copy link
Collaborator

srirambv commented May 4, 2017

Test plan

#8786 (comment)


  • Did you search for similar issues before submitting this one?
    Yes

  • Describe the issue you encountered:
    Saving a document doesn't show correct origin

  • Platform (Win7, 8, 10? macOS? Linux distro?):
    Alll

  • Brave Version (revision SHA):
    Brave 0.15.1
    rev ea5024d

  • Steps to reproduce:

    1. Enable payments
    2. Click on advance settings
    3. Save recovery file, download bar shows origin as file:///
    4. Similar issue when saving pdf file shows as blob:chrome-extension
  • Actual result:
    Saving a document doesn't show correct origin

  • Expected result:
    Should show the correct origin for files

  • Will the steps above reproduce in a fresh profile? If not what other info can be added?
    Yes

  • Is this an issue in the currently released version?
    Yes but since origin was not shown before had gone unnoticed

  • Can this issue be consistently reproduced?
    Yes

  • Extra QA steps:
    1.
    2.
    3.

  • Screenshot if needed:
    image
    image

  • Any related issues:
    Please change milestone if requried

@srirambv srirambv added this to the 0.15.4 milestone May 4, 2017
@kumarrishav
Copy link
Contributor

@srirambv So, what should be expected behaviour? whole path of the file ? like file:///Users/krishav/Library/Application%20Support/brave/brave_wallet_recovery.txt instead of brave_wallet_recovery.txt

@kumarrishav
Copy link
Contributor

kumarrishav commented May 5, 2017

what should be the origin in this scenario? file:///Users/krishav/Library/Application%20Support/brave/ledger-state.json or we should hide origin in these (file:/// and blob:chrome-extension) case?

Seems like it's the expected behaviour : https://github.com/brave/browser-laptop/blob/master/js/state/siteUtil.js#L800

@srirambv
Copy link
Collaborator Author

srirambv commented May 5, 2017

So, what should be expected behaviour? whole path of the file ? like file:///Users/krishav/Library/Application%20Support/brave/brave_wallet_recovery.txt instead of brave_wallet_recovery.txt

It should show path like this to keeping it in line for all downloads.
image

But for files downloaded from within the app we could skip it over. cc: @bbondy @bsclifton for thoughts

@kumarrishav kumarrishav self-assigned this May 5, 2017
@kumarrishav
Copy link
Contributor

Or can we have this in preference to disable the origin? Though it's good show the origin but as a user it doesn't much value (i think). @bbondy @bsclifton

@Jacalz
Copy link
Contributor

Jacalz commented May 6, 2017

I am with you on disabling it since it doesn't have much value to me 😉 @kumarrishav
My suggestion is having it in the generals tab right under default download path if so 👌

@bsclifton
Copy link
Member

@kumarrishav I can't think of any good reason we'd want to hide this. Because downloads can't always be trusted, showing the origin and download method (HTTP/HTTPS) is very important, IMO. Otherwise, all you know was that a download happened

For local files, we could choose to show something different... like "Local file" or show the folder it was downloaded to

cc: @diracdeltas for comment

@kumarrishav
Copy link
Contributor

@bsclifton agree with you. So, question is what content we should show for local files (file:/// , blob:chrome-extension)

@diracdeltas
Copy link
Member

FWIW, #8645 will allow us to load the PDF file directly instead of via a blob: URL, so that would partly solve the issue.

@diracdeltas
Copy link
Member

diracdeltas commented May 7, 2017

we could just replace local URL origins (file:, blob:, data:, chrome-extension:, chrome: etc.) with Local file instead of the full origin.

bsclifton added a commit that referenced this issue May 16, 2017
Auditors: @bbondy, @alexwykoff

Revert "Remove addEditBookmark.less"
This reverts commit c1c4437.

Revert "Add padding around showAllWrap button"
This reverts commit 4a9e270.

Revert "Shows notification caret on pinned tabs"
This reverts commit f829362.

Revert "unhide bitwarden password manager"
This reverts commit fc52e8c.

Revert "Saving a document doesn't show correct origin Fix #8698. Auditors @bsclifton @diracdeltas"
This reverts commit 277074e.

Revert "noscript exceptions from private tabs should not apply to regular tabs"
This reverts commit d6516f7.
@alexwykoff alexwykoff changed the title Saving a document doesn't show correct origin Saving a document doesn't show correct origin Jun 13, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.