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

Auto-purging "Saved site settings" resets Payments "Include" toggles #6708

Closed
analogist opened this issue Jan 18, 2017 · 12 comments
Closed

Auto-purging "Saved site settings" resets Payments "Include" toggles #6708

analogist opened this issue Jan 18, 2017 · 12 comments
Assignees
Labels
bug feature/rewards post-v1 We don't expect to be able to resolve this before releasing v1.0 with Brave Core (instead of Muon). release-notes/include wontfix

Comments

@analogist
Copy link

analogist commented Jan 18, 2017

Describe the issue you encountered:
It seems that the "Include" toggle state in Payments for individual sites does not survive browser restarts, if "Saved site settings and permissions" is set to autopurge on closing Brave.

Expected behavior:
I'm actually not sure if this is a technical limitation, but to me it seems like user settings in the about:* domains should survive purge-on-close.

  • Platform (Win7, 8, 10? macOS? Linux distro?):
    Windows 10 (v 1607)

  • Brave Version (revision SHA):
    0.12.15

  • Steps to reproduce:

    1. Turn on autopurge "Saved site settings and permissions" in "Security"/"Private Data"
    2. Turn on Brave Payments and form a payment ledger
    3. Toggle off some sites in the ledger with the "Include" toggle
    4. Restart Brave
    5. All sites are toggled back "on" in the "Include" list.

This issue does not seem to occur if "Saved site settings and permissions" is not set to autopurge.

@analogist analogist changed the title Auto-purging "Saved site settings" resets Payments ledger "Include" toggles Auto-purging "Saved site settings" resets Payments "Include" toggles Jan 18, 2017
@mrose17
Copy link
Member

mrose17 commented Jan 19, 2017

@bradleyrichter - is this the intended behavior? my thinking is that the behavior is consistent, but may not be desirable. what's your thinking?

@bradleyrichter
Copy link
Contributor

My initial thinking and current thinking is that these MUST be separated because it will always surprise people. I think you need 2 options:

The standard and Brave specific:

image

(not sure if the last switch should combine those data types but it would be more usable. )

@mrose17 mrose17 added this to the 1.0.0 milestone Jan 20, 2017
@mrose17 mrose17 added the bug label Jan 20, 2017
@mrose17 mrose17 modified the milestones: 0.13.3, 1.0.0 Jan 31, 2017
@mrose17 mrose17 modified the milestones: 0.13.5, 0.13.4 Feb 14, 2017
@diracdeltas
Copy link
Member

diracdeltas commented Mar 11, 2017

@bradleyrichter 'brave shields' is essentially the same as 'saved site settings'. if you want shields to be tied with brave payments, then 'saved site settings and permissions' should probably just be 'saved site permissions'.

@bsclifton
Copy link
Member

moving to 0.13.7

@bsclifton bsclifton modified the milestones: 0.13.7, 0.13.6 Mar 14, 2017
@diracdeltas
Copy link
Member

should the 'Brave payments data' switch clear all Brave payments data (resetting it to the state before you turned on Brave payments)?

@mrose17
Copy link
Member

mrose17 commented Apr 5, 2017

the only issue with that is the loss of the brave wallet, if the user didn't back up the recovery keys... i suspect it would be better to trim everything back in the synopsis file, most of the ledger state file, and then set the ledger setting to off along with deleting all the site-specific ledger settings...

@mrose17
Copy link
Member

mrose17 commented Apr 6, 2017

here is my suggestion:

  1. add the additional option "Brave shields and payment data" as shown in https://cloud.githubusercontent.com/assets/13509546/22130115/12199528-de5f-11e6-96f6-bb67c71c403c.png

  2. exclude any key starting with ledger from being deleted when "Saved site settings and permissions" is cleared.

  3. when "Brave shields and payment data" is cleared, reset ballots and transactions, and remove paymentInfo from ledger-state

  4. when "Brave shields and payment data" is cleared, reset ledger-scores, ledger-publisher and ledger-log

  5. when "Brave shields and payment data" is cleared, reset publishers from ledger-publishers

tasks 2-4 can be accomplished by modifying the correct data structures in app/ledger.js and then calling atomicWriter...

@mrose17 mrose17 modified the milestones: 0.14.3, 0.14.2 Apr 6, 2017
@diracdeltas
Copy link
Member

shields should not be tied to payments data IMO. if you are clearing site permissions, you are more likely to want to clear shield settings than if you are clearing payments data.

@mrose17
Copy link
Member

mrose17 commented Apr 6, 2017

@bradleyrichter - i agree with @diracdeltas comment, we probably need two more checkboxes, not just one...

@bsclifton bsclifton added needs-owner ♞ This issue is tagged for an upcoming release but has no owner. and removed needs-owner ♞ This issue is tagged for an upcoming release but has no owner. labels Apr 7, 2017
@alexwykoff alexwykoff modified the milestones: Backlog, 0.15.1 Apr 18, 2017
@mrose17 mrose17 modified the milestones: 1.1.0, Backlog Apr 20, 2017
@mrose17 mrose17 removed the needs-owner ♞ This issue is tagged for an upcoming release but has no owner. label Apr 20, 2017
@bsclifton
Copy link
Member

bsclifton commented Apr 22, 2017

Prework for this is done and can be found in the issues-6708 branch

@NejcZdovc NejcZdovc modified the milestones: 0.15.4, 1.1.0 May 4, 2017
@alexwykoff alexwykoff modified the milestones: 0.16.200, 0.16.100 May 23, 2017
@alexwykoff alexwykoff modified the milestones: 0.19.x (Nightly Channel), 0.18.x (Developer Channel) Jun 13, 2017
@alexwykoff alexwykoff modified the milestones: 0.21.x (Nightly Channel), 0.19.x (Beta Channel) Jul 18, 2017
@ghost ghost removed this from the 0.21.x (Nightly Channel) milestone Sep 26, 2017
@LaurenWags
Copy link
Member

@bsclifton bsclifton added this to the Triage Backlog milestone Nov 27, 2017
@NejcZdovc NejcZdovc added the post-v1 We don't expect to be able to resolve this before releasing v1.0 with Brave Core (instead of Muon). label Apr 6, 2018
@NejcZdovc
Copy link
Contributor

per discussion with @diracdeltas we are pushing this after 1.0

@bsclifton bsclifton removed this from the Triage Backlog milestone Sep 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug feature/rewards post-v1 We don't expect to be able to resolve this before releasing v1.0 with Brave Core (instead of Muon). release-notes/include wontfix
Projects
None yet
Development

No branches or pull requests

9 participants