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.

Sign up for GitHub

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

Jump to bottom

Favicons of brave welcome and rewards page are white in nightly and master on macOS #3933

Closed
yrliou opened this issue Mar 28, 2019 · 7 comments · Fixed by brave/brave-core#2494
Closed

Favicons of brave welcome and rewards page are white in nightly and master on macOS #3933

yrliou opened this issue Mar 28, 2019 · 7 comments · Fixed by brave/brave-core#2494
Assignees
@mkarolin
Labels
about-pages/welcome bug design A design change, especially one which needs input from the design team OS/Linux OS/macOS priority/P2 A bad problem. We might uplift this to the next planned release. QA Pass-Linux QA Pass-macOS QA/Test-Plan-Specified QA/Yes release-notes/exclude
Milestone
0.65.x - Release ...

Comments

@yrliou
Copy link
Member

yrliou commented Mar 28, 2019 •
edited
Loading

Description

favicon of brave://welcome in nightly channel seems off in dark mode, looks normal in light mode though.
btw I'm using macOS, didn't try other platforms.

Steps to Reproduce

Install nightly or run a local build based on master and visit brave://welcome

Actual result:

Screen Shot 2019-03-28 at 4 33 31 PM

Expected result:

Screen Shot 2019-03-28 at 4 34 04 PM

Reproduces how often:

Every time.

Brave version (brave://version info)

0.64.23 Chromium: 73.0.3683.75 (Official Build) nightly(64-bit)

Reproducible on current release:

Not reproducible in current dev channel
(0.63.20 Chromium: 73.0.3683.75 (Official Build) dev(64-bit))

Website problems only:

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

cc @cezaraugusto @petemill
@bsclifton bsclifton added design A design change, especially one which needs input from the design team bug labels Mar 28, 2019
@yrliou
Copy link
Member Author

yrliou commented Mar 29, 2019

I just saw the same issue with brave://rewards in our nightly.
Screen Shot 2019-03-28 at 5 40 51 PM

@srirambv
Copy link
Contributor

Could be macOS issue. Not seeing the white icon on Windows

@yrliou yrliou changed the title Favicon of brave welcome page is white in nightly and master Favicons of brave welcome and rewards page are white in nightly and master on macOS Mar 29, 2019
@NejcZdovc
Copy link
Contributor

NejcZdovc commented Mar 29, 2019 •
edited
Loading

saw the same problem, thought that this was intentional

@rebron rebron added OS/macOS priority/P3 The next thing for us to work on. It'll ride the trains. priority/P2 A bad problem. We might uplift this to the next planned release. and removed priority/P3 The next thing for us to work on. It'll ride the trains. labels Mar 29, 2019
@kjozwiak
Copy link
Member

Reproduced on Nightly as well 👍 Pretty easy to reproduce. Used the following build:

Brave 0.64.23 Chromium: 73.0.3683.75 (Official Build) nightly(64-bit)
Revision 909ee014fcea6828f9a610e6716145bc0b3ebf4a-refs/branch-heads/3683@{#803}
OS Mac OS X

STR:

  • Enable the Dark theme via brave://settings or under macOS if you're using Same as MacOS
  • visit about brave:// website and noticed the incorrect icons being used as @yrliou mentioned above

@NejcZdovc NejcZdovc mentioned this issue May 22, 2019
Closed
@rossmoody
Copy link
Contributor

Code in here I can't entirely understand but think it might be relevant so passing it along: https://cs.chromium.org/chromium/src/chrome/browser/favicon/favicon_utils.cc?g=0

@mkarolin mkarolin self-assigned this May 24, 2019
@mkarolin mkarolin mentioned this issue May 24, 2019
Merged
28 tasks
mkarolin added a commit to brave/brave-core that referenced this issue May 24, 2019
@mkarolin
Fixes tab icons for welcome and rewards pages on MacOS in dark theme.
bdd0546 
Fixes brave/brave-browser#3933

Patches chrome/browser/ui/views/tabs/tab_icon.cc's to use our version of
ShouldThemifyFaviconForUrl function which adds welcome and rewards pages
to the list of pages for which the favicon should not be "themified".
@mkarolin mkarolin added this to the 0.67.x - Nightly milestone May 24, 2019
This was referenced May 25, 2019
Merged
Merged
@kjozwiak
Copy link
Member

Adding OS/Linux as per #4535.

@LaurenWags
Copy link
Member

LaurenWags commented May 29, 2019 •
edited by btlechowski
Loading

Verified passed with

Brave 0.65.113 Chromium: 75.0.3770.38 (Official Build) beta(64-bit)
Revision 3860105745f2b12537da9e9f048f14c3f52ba970-refs/branch-heads/3770@{#618}
OS Mac OS X

Verification passed on

Brave 0.65.114 Chromium: 75.0.3770.38 (Official Build) (64-bit)
Revision 3860105745f2b12537da9e9f048f14c3f52ba970-refs/branch-heads/3770@{#618}
OS Ubuntu 18.04 LTS

image

image

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

@mkarolin mkarolin

Labels
about-pages/welcome bug design A design change, especially one which needs input from the design team OS/Linux OS/macOS priority/P2 A bad problem. We might uplift this to the next planned release. QA Pass-Linux QA Pass-macOS QA/Test-Plan-Specified QA/Yes release-notes/exclude
Projects
None yet
Milestone
0.65.x - Release (Release Channel)
Development

Successfully merging a pull request may close this issue.

Fixes tab icons for welcome and rewards pages on MacOS' dark theme. brave/brave-core