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

Importing bookmarks on a new install automatically adds the top site tiles #6217

Closed
srirambv opened this issue Dec 14, 2016 · 10 comments
Closed

Comments

@srirambv
Copy link
Collaborator

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

Describe the issue you encountered:
Importing bookmarks on a new install automatically adds the top site tiles

Expected behavior:
Should add top tiles only if the user visits the bookmarked sites

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

  • Brave Version:
    0.12.15

  • Steps to reproduce:

    1. Do a clean install of brave
    2. Once the browser opens after install import bookmarks from HTML file
    3. Bookmarks get added as top tiles automatically
  • Screenshot if needed:
    toptiles

  • Any related issues:
    cc: @cezaraugusto
    Please change the milestone if required

@luixxiul
Copy link
Contributor

I don't hate that.

@bsclifton
Copy link
Member

Closing as this was fixed with #6655 and merged into 0.13.1-branch

@cezaraugusto
Copy link
Contributor

Steps to reproduce:

  1. On a fresh session, remove some default topSites
  2. Import bookmarks
  3. Imported bookmarks shouldn't populate topSites grid

@aekeus
Copy link
Member

aekeus commented Feb 4, 2017

The tiles were shown on the newtab page after following the steps above.

@cezaraugusto
Copy link
Contributor

I couldn't reproduce on macOS running 0.13.2dev-RC3. Also tested on master latest (54ac442).

imported_bm_newtab

@cezaraugusto
Copy link
Contributor

Also test is still passing:

npm run test -- --grep="does not include imported bookmarks"

Do we at some point changed how we check if bookmark is imported (lastAccessedTime === 0)?

/cc @darkdh

@darkdh
Copy link
Member

darkdh commented Feb 4, 2017

I can't reproduce either. And I intentionally clean the history before importing and nothing shows up in tiles.

@bbondy
Copy link
Member

bbondy commented Feb 5, 2017

since 3 reports of it working I think this is probably a testing problem, but please try again @aekeus. Possibly what happened was you did the update manual test before this one?
If you can still reproduce though please re-open and we'll just move it into 0.13.3.

@srirambv
Copy link
Collaborator Author

srirambv commented Feb 5, 2017

Works fine on Windows
6217

@aekeus
Copy link
Member

aekeus commented Feb 6, 2017

Confirmed, this was a testing issue. Sorry for the confusion.

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