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

Restore all from deleted publisher doesn't restore all #1441

Closed
srirambv opened this issue Oct 5, 2018 · 10 comments
Closed

Restore all from deleted publisher doesn't restore all #1441

srirambv opened this issue Oct 5, 2018 · 10 comments

Comments

@srirambv
Copy link
Contributor

srirambv commented Oct 5, 2018

Description

Restore all doesn't get removed after restoring all deleted sites

Steps to Reproduce

  1. Enable Rewards on 0.55.1
  2. Visit 8-10 sites so that its listed on the auto-contribute list
  3. Delete 2-3 sites from list
  4. Click on Show all to see the full list of publishers
  5. Click on Restore All from the publisher window. Removed site gets added but still shows 1 excluded site

Actual result:

restore all1

Expected result:

Restore all should not show any value for excluded list. Defeats the purpose of restore all

Reproduces how often:

Easy

Brave version (chrome://version info)

Brave 0.55.11 Chromium: 70.0.3538.35 (Official Build) beta (64-bit)
Revision 28dcb499844fa40c28d5f62e337876cb936f79f5-refs/branch-heads/3538@{#678}
OS Linux

Reproducible on current release:

  • Does it reproduce on brave-browser dev/beta builds? Yes on beta
  • Does it reproduce on browser-laptop? No.

Website problems only:

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

Additional Information

cc: @LaurenWags @kjozwiak @NejcZdovc

@srirambv srirambv added this to the 1.x Backlog milestone Oct 5, 2018
@LaurenWags
Copy link
Member

Did not reproduce on macOS. After clicking 'Restore All' link the entire line went away and all sites were restored to the list.

Brave 0.55.11 Chromium: 70.0.3538.35 (Official Build) beta(64-bit)
Revision 28dcb499844fa40c28d5f62e337876cb936f79f5-refs/branch-heads/3538@{#678}
OS Mac OS X

1441

@jasonrsadler
Copy link

jasonrsadler commented Oct 9, 2018

@LaurenWags I think it happens when we have 11 or more sites.

It seems to restore enough to make current list 10.

@LaurenWags
Copy link
Member

@jasonrsadler hm, I tried with 12 sites on dev (I can't get more than 10 on Beta due to another issue) and I'm still not able to repro

Brave 0.56.2 Chromium: 70.0.3538.35 (Official Build) dev (64-bit)
Revision 28dcb499844fa40c28d5f62e337876cb936f79f5-refs/branch-heads/3538@{#678}
OS Mac OS X

1441-part2

@jasonrsadler
Copy link

@LaurenWags disregard. I was reproducing on beta. Just tried again on dev macOS and seems to be working.

@srirambv
Copy link
Contributor Author

Found the actual steps which causes this issue

  1. Visit 7-8 sites to populate the table
  2. Exclude one site (say github.com) from the table
  3. Ensure the exclude site shows as 1 and restore all
  4. Revisit github.com in a new tab or the existing tab
  5. Once min time is met, github.com gets added back to the list
  6. Exclude list still shows 1 excluded site (github.com from step 2)
  7. Restore all doens't remove the count of excluded sites since github.com got added back to the list from step 5.

Either publisher should not be added back to the list until its removed from the excluded list or if its getting added back automatically it should not be counted in excluded list.

cc: @NejcZdovc @bbondy @rebron this needs to be prioritized for release build

@LaurenWags
Copy link
Member

@srirambv @NejcZdovc @rebron Reproduced with steps from #1441 (comment)

Brave 0.55.12 Chromium: 70.0.3538.45 (Official Build) beta(64-bit)
Revision cbdc32e4334458954e9def214d7e5fa1ca1960eb-refs/branch-heads/3538@{#830}
OS Mac OS X

@NejcZdovc NejcZdovc modified the milestones: 1.x Backlog, 0.57.x - Dev Oct 22, 2018
@jasonrsadler
Copy link

jasonrsadler commented Oct 28, 2018

@srirambv @LaurenWags Can you verify if this could be related or the cause?
#1873

When you are excluding sites are you clicking the 'X' more than once before the site has a chance to go away (It goes away really fast on a release build but this reproduces easily on a dev build)

@srirambv I notice in your graphic above it shows 10 listed with 1 excluded = 11 but as you're excluding, the total number of sites (excluded + non-excluded) goes up.

@bbondy bbondy modified the milestones: 0.57.x - Dev, 1.x Backlog Oct 30, 2018
@LaurenWags
Copy link
Member

@jasonrsadler I definitely did not click x more than once, however I just tried the steps from #1441 (comment) and was unable to reproduce the issue using

Brave 0.56.7 Chromium: 70.0.3538.77 (Official Build) beta(64-bit)
Revision 0f6ce0b0cd63a12cb4eccea3637b1bc9a29148d9-refs/branch-heads/3538@{#1039}
OS Mac OS X

@NejcZdovc
Copy link
Contributor

Will close this one and let's retest it in the latest master, where brave/brave-core#797 will land as well

@btlechowski
Copy link

btlechowski commented Dec 12, 2018

Verification passed on

Brave 0.58.11 Chromium: 71.0.3578.80 (Official Build) beta (64-bit)
Revision 2ac50e7249fbd55e6f517a28131605c9fb9fe897-refs/branch-heads/3578@{#860}
OS Windows 7 x64

Used Test Plan from brave/brave-core#797 and STR from OP and comments.

Verification passed on

Brave 0.58.11 Chromium: 71.0.3578.80 (Official Build) beta (64-bit)
Revision 2ac50e7249fbd55e6f517a28131605c9fb9fe897-refs/branch-heads/3578@{#860}
OS Linux

Verified passed with

Brave 0.58.12 Chromium: 71.0.3578.80 (Official Build) (64-bit)
Revision 2ac50e7249fbd55e6f517a28131605c9fb9fe897-refs/branch-heads/3578@{#860}
OS Mac OS X

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment