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

[Desktop] brave://bookmarks tab icon uses a star icon instead of Brave bookmarks icon - follow up to 2494 #8492

Closed
LaurenWags opened this issue Mar 2, 2020 · 6 comments · Fixed by brave/brave-core#5709

Comments

@LaurenWags
Copy link
Member

Description

Per #2494 (comment) we should also address the favicon in the brave://bookmarks tab and use our own branded icon instead of the star icon.

Steps to Reproduce

  1. Navigate to brave://bookmarks
  2. Look at tab icon

Actual result:

See star icon on tab
Screen Shot 2020-03-02 at 3 35 41 PM

Expected result:

Tab favicon should show our own branded icon, not the star icon

Reproduces how often:

easily

Brave version (brave://version info)

Brave 1.5.105 Chromium: 80.0.3987.122 (Official Build) beta (64-bit)
Revision cf72c4c4f7db75bc3da689cd76513962d31c7b52-refs/branch-heads/3987@{#943}
OS macOS Version 10.14.6 (Build 18G3020)

Version/Channel Information:

  • Can you reproduce this issue with the current release? yes
  • Can you reproduce this issue with the beta channel? yes
  • Can you reproduce this issue with the dev channel? yes
  • Can you reproduce this issue with the nightly channel? yes

Other Additional Information:

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

Miscellaneous Information:

@rebron
Copy link
Collaborator

rebron commented Apr 17, 2020

@karenkliu Can we get an icon for this one? This one has been bothering me too.

@karenkliu
Copy link

karenkliu commented Apr 17, 2020

@rebron This issue is a dupe of #7312 and I already posted the icon for it

@rebron rebron reopened this Apr 17, 2020
@rebron
Copy link
Collaborator

rebron commented Apr 17, 2020

Screen Shot 2020-04-17 at 2 47 03 PM

Need icon for above similar to:

Screen Shot 2020-04-17 at 2 47 45 PM

I think #7312 are for the folder icons no?

@karenkliu
Copy link

@rebron Oops, sorry you're right, that's a different issue.

Actually the one you need in the tab title is probably this icon:

Screen Shot 2020-04-17 at 3 12 19 PM

Here you go!
bookmark icon.zip

@simonhong
Copy link
Member

WebUI's favicon is determined by ChromeWebUIControllerFactory::GetFaviconResourceBytes()

@kjozwiak
Copy link
Member

kjozwiak commented Jun 16, 2020

Verification PASSED on macOS 10.15.5 x64 using the following build:

Brave | 1.11.64 Chromium: 83.0.4103.97 (Official Build) dev (64-bit)
-- | --
Revision | 326d148b9655369b86498d9ecca39f63dd2bdd2d-refs/branch-heads/4103@{#657}
OS | macOS Version 10.15.5 (Build 19F101)
1.10.90 CR: 83.0.4103.97 (Old) 1.11.64 CR: 83.0.4103.97 (New)
Screen Shot 2020-06-16 at 12 08 33 AM Screen Shot 2020-06-16 at 12 09 00 AM

Verification passed on

Brave | 1.11.65 Chromium: 83.0.4103.97 (Official Build) dev (64-bit)
-- | --
Revision | 326d148b9655369b86498d9ecca39f63dd2bdd2d-refs/branch-heads/4103@{#657}
OS | Windows 10 OS Version 1903 (Build 18362.30)

  • Verified that bookmark icon shows brave branded favicon
  • Installed 1.10.x and upgraded profile to 1.11.x and verified that bookmark icon is replaced with brave branded favicons
    image

Verified passed with

Brave	1.11.80 Chromium: 83.0.4103.116 (Official Build) dev (64-bit)
Revision	8f0c18b4dca9b6699eb629be0f51810c24fb6428-refs/branch-heads/4103@{#716}
OS	Linux
  • Confirmed that brave://bookmarks tab shows Brave favicon

Screen Shot 2020-06-29 at 9 14 34 AM

@rebron rebron changed the title brave://bookmarks tab icon uses star icon instead of Brave bookmarks icon - follow up to 2494 [Desktop] brave://bookmarks tab icon uses a star icon instead of Brave bookmarks icon - follow up to 2494 Jul 14, 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.

7 participants