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

Correct site favicons are not displayed in the Shields panel #24954

Closed
MadhaviSeelam opened this issue Aug 24, 2022 · 2 comments · Fixed by brave/brave-core#14816
Closed

Correct site favicons are not displayed in the Shields panel #24954

MadhaviSeelam opened this issue Aug 24, 2022 · 2 comments · Fixed by brave/brave-core#14816

Comments

@MadhaviSeelam
Copy link

Description

Steps to Reproduce

  1. Install 1.45.1
  2. Launch Brave
  3. visit https://cnn.com
  4. click Shields icon
  5. click learn more link in the Shields panel

Actual result:

Incorrect icons rendered

Ex1 Ex2
cnnsite privacy

Expected result:

CNN favicon should render for CNN.com
When clicked on Learn more link Brave favicon should render in the Shields panel for https://brave.com/privacy-features/ page

Reproduces how often:

Easily

Brave version (brave://version info)

Brave 1.45.1 Chromium: 105.0.5195.37 (Official Build) nightly (64-bit)
Revision 5f67e9f258cc28ab970e57572290edf1863ee3c5-refs/branch-heads/5195@{#621}
OS Windows 11 Version 21H2 (Build 22000.856)

Version/Channel Information:

  • Can you reproduce this issue with the current release? N/A
  • Can you reproduce this issue with the beta channel? N/A
  • 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:

@mkarolin mkarolin self-assigned this Aug 24, 2022
mkarolin added a commit to brave/brave-core that referenced this issue Aug 25, 2022
Fixes brave/brave-browser#24954

The chrome://favicon2 syntax has changed to camelCase.

Chromium change:

https://source.chromium.org/chromium/chromium/src/+/78e45e119c9aca1eabf7e0b23764d1923f25a720

commit 78e45e119c9aca1eabf7e0b23764d1923f25a720
Author: Solomon Kinard <solomonkinard@chromium.org>
Date:   Mon Jun 13 23:45:53 2022 +0000

    Favicon: Use camelCase params instead of snake_case in JS context

    Doc:
    https://docs.google.com/document/d/1ksvSq1zF-9jYSfGOvNluyoTV0Ud9xl6r9HiAKONqmz8

    Context: crrev.com/c/3673461

    Bug: 104102
@brave-builds brave-builds added this to the 1.45.x - Nightly milestone Aug 25, 2022
@GeetaSarvadnya
Copy link

Issue is NOT reproducible in RC - 1.42.x hence adding the regression label

@GeetaSarvadnya
Copy link

Verification PASSE on

Brave | 1.43.82 Chromium: 105.0.5195.58 (Official Build) (64-bit)
-- | --
Revision | 1907e1693887af4463221a10247360b7a056db49-refs/branch-heads/5195_47@{#3}
OS | Windows 10 Version 21H2 (Build 19044.1889)

Ensured that correct site favicon is shown in the shield panel
Staging:

Example Example Example Example Example Example
image image image image image image

Production

Example Example Example Example Example Example
image image image image image image

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