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

Clearing "History" doesn't remove "Top Sites" on the new tab page #9929

Closed
sayedarifuddin opened this issue May 25, 2020 · 16 comments · Fixed by brave/brave-core#6047
Closed

Comments

@sayedarifuddin
Copy link

image
Version 1.9.72 Chromium: 81.0.4044.138 (Official Build) (64-bit)
image

@markwylde
Copy link

markwylde commented Jun 20, 2020

This is still not fixed in:
Version 1.10.93 Chromium: 83.0.4103.106 (Official Build) (64-bit)

Can we at least warn users we are not respecting their privacy until a fix can be put in, or ideally remove top sites until it can be implemented with the users privacy settings in mind?

@markwylde
Copy link

markwylde commented Jun 27, 2020

This might actually be fixed in the latest update. I've not managed to recreate this issue today on:
Version 1.10.97 Chromium: 83.0.4103.116 (Official Build) (64-bit)

@markwylde
Copy link

markwylde commented Jun 27, 2020

Nope, just happened again:
Screen Shot 2020-06-27 at 8 59 53 pm

I can't find where that URL is stored however. It doesn't seem to be anywhere (in text) in:
/Users/mark/Library/Application Support/BraveSoftware

@bsclifton
Copy link
Member

cc: @rebron - we probably should make it visible on the Clear on exit modal which group clears top sites (or we could give it its own entry)

@markwylde
Copy link

@BrendanEich can we please, as a stop gap, warn people that wiping their history is not wiping everything?

@MuhAssar
Copy link

MuhAssar commented Jul 6, 2020

someone posted this issue on hacker news
https://news.ycombinator.com/item?id=23746117

@RickyNotaro
Copy link

Someone posted on Hacker News that someone posted on the GitHub issue that someone posted the GitHub issue on Hacker News.

@simonhong
Copy link
Member

simonhong commented Jul 7, 2020

NTP gets topsites data from browser's history and manages them(site url, favicon cache url and etc...) to its own place(localStorage) in here.
Because of this, NTP shows previous topsite tiles after clearing history. (But favicon is not visible because NTP uses cached in history.)
To clear topsites when history is cleared, its data in localStorage should be reset.
cc: @bsclifton @petemill

@simonhong simonhong self-assigned this Jul 7, 2020
@simonhong
Copy link
Member

I'm trying to fix this by clearing topsites data implicitely when user clears history. WDYT? @rebron @bsclifton

@bsclifton
Copy link
Member

bsclifton commented Jul 7, 2020

@simonhong that should work perfectly. We will allow those to be edited (we have an issue for it), but at the moment it's tied directly to history (only real action you can do is move, pin, and close)

Marked as P2 since there is a leak (localStorage)

@bsclifton bsclifton added priority/P2 A bad problem. We might uplift this to the next planned release. feature/history labels Jul 7, 2020
@simonhong
Copy link
Member

@bsclifton Ok. Working on it :)

@Joshfindit
Copy link

As a user, I’m now concerned that there are other places where urls and/or history are kept.

@bsclifton
Copy link
Member

@Joshfindit top sites should be the only place where we stored URLs - which with localStorage it would be tied to the Brave extension's localStorage. Clearing cookies should be clearing it - but the problem is if you pick clear history, this should be wiped too

There aren't any other areas we're aware of- but might be good to do an audit
cc: @diracdeltas

@n-e-r-u
Copy link

n-e-r-u commented Jul 8, 2020

Yes please fix this. My student seen the pornhub icon on top sites.

@n-e-r-u
Copy link

n-e-r-u commented Jul 8, 2020

@n-e-r-u Why don't you at least use the Incognito mode?

It was just a joke. At least, i imagine people complaining about this issue because of porn website in top sites :D

@GeetaSarvadnya
Copy link

GeetaSarvadnya commented Aug 18, 2020

Verification passed on


Brave | 1.13.73 Chromium: 84.0.4147.125 (Official Build) dev (64-bit)
-- | --
Revision | d0784639447f2e10d32ebaf9861092b20cfde286-refs/branch-heads/4147@{#1059}
OS | Windows 10 OS Version 1903 (Build 18362.1016)

Scenario 1:
NTP with some top sites
image

  • Verified that the deleted site cnn.com from the brave://histrory is removed from NTP top sites list
    image
  • Verified that the deleted pinned site theverge.com from the brave://histrory is removed from NTP top sites list
    image
    image
  • Verified that the top sites are removed from NTP's when whole history is removed from brave://history
    image

Scenario 2: - launch profile with SR code

  • Verified that the NTP's have SR images along with SR top sites pinned
    image

  • Verified that the history is empty in SR profile
    image


Verification passed on

Brave 1.13.77 Chromium: 85.0.4183.69 (Official Build) dev (64-bit)
Revision 4554ea1a1171bd8d06951a4b7d9336afe6c59967-refs/branch-heads/4183@{#1426}
OS Ubuntu 18.04 LTS

Scenario 1:
4 sites:
image
Deleted cnn from history
image
Verified cnn.com top site is deleted too
image

  • Verified that the deleted pinned site interia from the brave://histrory is removed from NTP top sites list
    image
  • Verified that the top sites are removed from NTP's when whole history is removed from brave://history
    image

Scenario 2: - launch profile with SR code
SR is not supported on linux


Verified passed with

Brave	1.13.79 Chromium: 85.0.4183.69 (Official Build) dev (64-bit)
Revision	4554ea1a1171bd8d06951a4b7d9336afe6c59967-refs/branch-heads/4183@{#1426}
OS	macOS Version 10.14.6 (Build 18G3020)
Verified plan 1 from https://github.com/brave/brave-core/pull/6047

Step 2 - top site tiles populated:
step 2 - top sites populated

Step 4 - after one site is removed from history, confirmed it is not shown in top site tiles any more:
step 4 - one site deleted from history

Step 5 - pinned one top site tile:
step 5 - one top site piined

Step 7 - after the pinned site is removed from history, confirmed it is not shown in top site tiles any more:
step 7 - pinned site removed

Step 8 - cleared history
step 8 - history cleared

Step 9 - confirmed all top site tiles are removed
step 9 - all top site tiles gone

Additionally:

  • After completing plan 1 above, confirmed that top sites are able to be populated again (visited some previous and new sites).
  • Confirmed if you have History set to clear "On Exit", the top site tiles are cleared when you relaunch.
Verified plan 2 from https://github.com/brave/brave-core/pull/6047

Confirmed SR install (used TECHNIK) top site tiles are populated without any history:
scenario 2

@rebron rebron changed the title Clearing "History" doesn't remove "Top Sites" in the homepage. Clearing "History" doesn't remove "Top Sites" on the new tab page Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.