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

Favicons not loaded when bookmarks imported from browser #4882

Closed
srirambv opened this issue Oct 18, 2016 · 3 comments · Fixed by brave/muon#74 or #4930
Closed

Favicons not loaded when bookmarks imported from browser #4882

srirambv opened this issue Oct 18, 2016 · 3 comments · Fixed by brave/muon#74 or #4930
Assignees
@darkdh
Labels
feature/bookmarks feature/importer misc/favicon QA/checked-Linux QA/checked-macOS QA/checked-Win64
Milestone
0.12.6dev

Comments

@srirambv
Copy link
Collaborator

Did you search for similar issues before submitting this one?

Describe the issue you encountered:
Favicons not loaded when bookmarks imported from browser

Expected behavior:
Should load favicons irrespective how the data is imported

  • Platform (Win7, 8, 10? macOS? Linux distro?):
    Windows 10 x64
  • Brave Version:
    0.12.5 RC4
  • Steps to reproduce:
    1.
    2.
    3.
  • Screenshot if needed:
    favicon
  • Any related issues:
@srirambv srirambv added this to the 0.12.6dev milestone Oct 18, 2016
@darkdh
Copy link
Member

darkdh commented Oct 18, 2016

they do exist in Bookmarks Manager but not Bookmarks Toolbar

@darkdh
Copy link
Member

darkdh commented Oct 18, 2016

And chrome doesn't have favicon callback, that's on me.

darkdh added a commit to darkdh/browser-laptop that referenced this issue Oct 19, 2016
@darkdh
Import favicon fix:
d804e39 
1. Iterate all urls using the favicon
2. Adjust made-up-favicon condition and override old made-up-favicon url

fix brave#4882

Auditors: @bridiver, @bbondy

Test Plan:
1. Import bookmarks from firefox on brave 0.12.3
2. Import bookmarks from firefox on brave contains this commit
3. The bookmarks under "Mozilla Firefox" should have favicons

1. Import bookmarks from any browsers
2. The imported bookmarks should have favicons

1. Import bookmarks from html bookmarks file
2. The imported bookmarks should have favicons
@darkdh darkdh mentioned this issue Oct 19, 2016
Merged
4 tasks
darkdh added a commit to brave/muon that referenced this issue Oct 19, 2016
@darkdh
Add favicon for chrome bookmarks import
5ed1ed2 
fix brave/browser-laptop#4882

Auditors: @bridiver, @bbondy
@darkdh darkdh mentioned this issue Oct 19, 2016
Merged
@darkdh darkdh reopened this Oct 19, 2016
@srirambv
Copy link
Collaborator Author

@darkdh Manually added bookmarks on Chrome which are imported is not generating the favicons

importfavicons

This was referenced Oct 20, 2016
Closed
Closed
@srirambv srirambv mentioned this issue Oct 20, 2016
Closed
@alexwykoff alexwykoff mentioned this issue Oct 22, 2016
Closed
@bsclifton bsclifton mentioned this issue Mar 15, 2017
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@darkdh darkdh

Labels
feature/bookmarks feature/importer misc/favicon QA/checked-Linux QA/checked-macOS QA/checked-Win64
Projects
None yet
Milestone
0.12.6dev
Development

Successfully merging a pull request may close this issue.

Add favicon for chrome bookmarks import brave/muon
Import favicon fix: darkdh/browser-laptop
4 participants
@alexwykoff @luixxiul @darkdh @srirambv