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

verified sites not showing correct favicon in panel - follow up to 3525 #3783

Closed
LaurenWags opened this issue Mar 19, 2019 · 12 comments · Fixed by brave/brave-core#2077
Closed

Comments

@LaurenWags
Copy link
Member

Description

Found while testing #3525

Verified publisher sites (non YT or Twitch) are not displaying favicon in the panel correctly.

Steps to Reproduce

  1. Clean profile.
  2. Enable Rewards
  3. Visit a verified site (brave.com, basicattentiontoken.org, ddg, etc)
  4. Wait a few seconds, open panel.

Actual result:

Favicon not shown. If you wait some time, after opening/closing panel the favicon will probably be shown.
Screen Shot 2019-03-19 at 1 19 38 PM

Expected result:

Favicon should be shown.
Screen Shot 2019-03-19 at 1 55 12 PM

Reproduces how often:

easily

Brave version (brave://version info)

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

Reproducible on current release: no

  • Does it reproduce on brave-browser dev/beta builds? Dev yes, beta no.

Website problems only:

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

Additional Information

If you run thru STR with a non-verified site (google, nyt) or a verified youtube or twitch pub, the favicon displays as expected.
Reproduced by @GeetaSarvadnya @kjozwiak on Win and Linux respectively

@LaurenWags
Copy link
Member Author

Also occurs when using staging environment (need to use a different set of verified sites for testing).

@btlechowski
Copy link

Reproduced on

Brave 0.63.14 Chromium: 73.0.3683.75 (Official Build) dev (64-bit)
Revision 909ee014fcea6828f9a610e6716145bc0b3ebf4a-refs/branch-heads/3683@{#803}
OS Windows 10 OS Build 17134.523

image

@NejcZdovc
Copy link
Contributor

@LaurenWags can you try this on the latest nightly. I can't reproduce on latest 0.64

@NejcZdovc NejcZdovc added needs-more-info The report requires more detail before we can decide what to do with this issue. and removed regression labels Mar 25, 2019
@LaurenWags
Copy link
Member Author

I don't know what version you tried @NejcZdovc but I reproduced with latest Nightly available for macOS

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

Screen Shot 2019-03-25 at 10 10 33 AM

@btlechowski
Copy link

@NejcZdovc I am able to reproduce on Windows nightly

Brave 0.64.8 Chromium: 73.0.3683.75 (Official Build) nightly (64-bit)
Revision 909ee014fcea6828f9a610e6716145bc0b3ebf4a-refs/branch-heads/3683@{#803}
OS Windows 10 OS Build 17134.523

image

@NejcZdovc
Copy link
Contributor

  1. can you please inspect this icon and pass url of it?
  2. is domain that you are on with www or without?
  3. if you reopen panel will this fix the problem?

@NejcZdovc NejcZdovc self-assigned this Mar 26, 2019
@NejcZdovc NejcZdovc added priority/P1 A very extremely bad problem. We might push a hotfix for it. and removed needs-more-info The report requires more detail before we can decide what to do with this issue. labels Mar 26, 2019
@NejcZdovc NejcZdovc added this to the 0.64.x - Nightly milestone Mar 26, 2019
NejcZdovc added a commit to brave/brave-core that referenced this issue Mar 26, 2019
@NejcZdovc
Copy link
Contributor

found the problem, this regressed with this one brave/brave-core#1649, so would be good to uplift fix to 0.62.

cc @brave/uplift-approvers

@kjozwiak
Copy link
Member

found the problem, this regressed with this one brave/brave-core#1649, so would be good to uplift fix to 0.62.

cc @brave/uplift-approvers

@NejcZdovc did this regress in 0.62.x? I can't seem to reproduce it using 0.62.41 Chromium: 73.0.3683.86 and I can reproduce it with both 0.63.20 Chromium: 73.0.3683.75 and 0.64.8 Chromium: 73.0.3683.75 using the STR that @LaurenWags mentioned under #3783 (comment).

@NejcZdovc
Copy link
Contributor

if that is the case then we can uplift only in 63

@LaurenWags
Copy link
Member Author

I have been unable to reproduce with most recent 0.62.x version on macOS

Brave 0.62.41 Chromium: 73.0.3683.86 (Official Build) beta(64-bit)
Revision f9b0bec6063ea50ce2b71f5b9abbae7beee319a6-refs/branch-heads/3683@{#858}
OS Mac OS X

@kjozwiak
Copy link
Member

kjozwiak commented Mar 28, 2019

Thanks for the double check @LaurenWags 👍

if that is the case then we can uplift only in 63

Sounds good! Thanks @NejcZdovc. Once the dev channel opens up for uplift, will go through with @brave/uplift-approvers and approve as it's a regression that needs to get resolved.

@LaurenWags
Copy link
Member Author

LaurenWags commented Apr 3, 2019

Verified passed with

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

Verification passed on

Brave 0.63.30 Chromium: 73.0.3683.75 (Official Build) beta (64-bit)
Revision 909ee014fcea6828f9a610e6716145bc0b3ebf4a-refs/branch-heads/3683@{#803}
OS Windows 10 OS Build 17134.523

Verification passed on

Brave 0.63.31 Chromium: 73.0.3683.75 (Official Build) beta (64-bit)
Revision 909ee014fcea6828f9a610e6716145bc0b3ebf4a-refs/branch-heads/3683@{#803}
OS Linux

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.

6 participants