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

Linux shows wrong version installed and wrong browser icon #712

Closed
srirambv opened this issue Aug 9, 2018 · 7 comments · Fixed by brave/brave-core#371
Closed

Linux shows wrong version installed and wrong browser icon #712

srirambv opened this issue Aug 9, 2018 · 7 comments · Fixed by brave/brave-core#371
Assignees
Labels

Comments

@srirambv
Copy link
Contributor

srirambv commented Aug 9, 2018

Description

Linux shows wrong version installed and wrong browser icon

Steps to Reproduce

  1. Manually install 0.53.1 via dpkg -i <0.53.1.deb>
  2. Download 0.53.2 deb file. Install it via Ubuntu Software package manager
  3. Software Package manager still shows 0.53.1 and shows Orange browser icon
  4. Launch browser, About Brave shows 0.53.2 which is the correct version
    Note: The wrong browser icon shows up on all versions, not specific to update scenario mentioned above

Actual result:

image
image
brave-linux

Expected result:

Should show correct version in package manager and should install correct browser icon

Reproduces how often:

100%

Brave version (about:brave info)

0.53.1 and 0.53.2

Reproducible on current release:

No browser-laptop shows correct version when installed and shows the correct browser icon

Website problems only:

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

Additional Information

cc: @bbondy marked the issue as blocker since its version mismatch. Please move to backlog if needed

@mbacchi
Copy link
Contributor

mbacchi commented Aug 20, 2018

After looking at this a little bit, I think we need to allow for dev/beta specific icons in the script chrome/installer/linux/common/installer.include just as Chrome does in this section:

https://github.com/chromium/chromium/blob/master/chrome/installer/linux/common/installer.include#L278-L284

They obviously have different icons for dev/beta and we should provide for this to integrate with the desktop. I will work on a patch soon.

@mbacchi mbacchi added the priority/P1 A very extremely bad problem. We might push a hotfix for it. label Aug 20, 2018
@mbacchi
Copy link
Contributor

mbacchi commented Aug 27, 2018

So this isn't as simple as I thought. I'm unsure how chromium produces dev/beta icons in their build, because they aren't in the repository. @simonhong do you have any ideas how we can mimic what they do for browser icons with dev/beta?

I see that in their packages they have differing icons depending on the channel:

[mbacchi@host Downloads]$ ls -ltr /opt/google/chrome-beta/*.png
-rw-r--r--. 1 root root  2514 Aug 22 23:03 /opt/google/chrome-beta/product_logo_64_beta.png
-rw-r--r--. 1 root root  1852 Aug 22 23:03 /opt/google/chrome-beta/product_logo_48_beta.png
-rw-r--r--. 1 root root  1194 Aug 22 23:03 /opt/google/chrome-beta/product_logo_32_beta.png
-rw-r--r--. 1 root root 11126 Aug 22 23:03 /opt/google/chrome-beta/product_logo_256_beta.png
-rw-r--r--. 1 root root   900 Aug 22 23:03 /opt/google/chrome-beta/product_logo_24_beta.png
-rw-r--r--. 1 root root   818 Aug 22 23:03 /opt/google/chrome-beta/product_logo_22_beta.png
-rw-r--r--. 1 root root   682 Aug 22 23:03 /opt/google/chrome-beta/product_logo_16_beta.png
-rw-r--r--. 1 root root  5295 Aug 22 23:03 /opt/google/chrome-beta/product_logo_128_beta.png
[mbacchi@host Downloads]$ ls -ltr /opt/google/chrome/*.png
-rw-r--r--. 1 root root  2786 Aug  7 20:20 /opt/google/chrome/product_logo_64.png
-rw-r--r--. 1 root root  2047 Aug  7 20:20 /opt/google/chrome/product_logo_48.png
-rw-r--r--. 1 root root  1289 Aug  7 20:20 /opt/google/chrome/product_logo_32.png
-rw-r--r--. 1 root root 13480 Aug  7 20:20 /opt/google/chrome/product_logo_256.png
-rw-r--r--. 1 root root   962 Aug  7 20:20 /opt/google/chrome/product_logo_24.png
-rw-r--r--. 1 root root   870 Aug  7 20:20 /opt/google/chrome/product_logo_22.png
-rw-r--r--. 1 root root   689 Aug  7 20:20 /opt/google/chrome/product_logo_16.png
-rw-r--r--. 1 root root  6184 Aug  7 20:20 /opt/google/chrome/product_logo_128.png

But in their source they don't have files with dev or beta in their name:

[mbacchi@host chromium]$ pwd
~/data/repos/chromium
[mbacchi@host chromium]$ find . -name product_logo*.png
./extensions/test/data/unpacker/good_package/product_logo_128.png
./extensions/test/data/unpacker/bad_zip/product_logo_128.png
./remoting/resources/product_logo_32.png
./components/resources/default_200_percent/chromium/product_logo.png
./components/resources/default_100_percent/chromium/product_logo.png
./chrome/android/java/res_chromium/drawable-xxhdpi/product_logo_name.png
./chrome/android/java/res_chromium/drawable-xhdpi/product_logo_name.png
./chrome/android/java/res_chromium/drawable-hdpi/product_logo_name.png
./chrome/android/java/res_chromium/drawable-mdpi/product_logo_name.png
./chrome/android/java/res_chromium/drawable-xxxhdpi/product_logo_name.png
./chrome/app/theme/chromium/product_logo_24.png
./chrome/app/theme/chromium/product_logo_22.png
./chrome/app/theme/chromium/product_logo_22_mono.png
./chrome/app/theme/chromium/product_logo_48.png
./chrome/app/theme/chromium/product_logo_128.png
./chrome/app/theme/chromium/product_logo_64.png
./chrome/app/theme/chromium/product_logo_256.png
./chrome/app/theme/default_200_percent/chromium/product_logo_16.png
./chrome/app/theme/default_200_percent/chromium/product_logo_name_22.png
./chrome/app/theme/default_200_percent/chromium/product_logo_name_48.png
./chrome/app/theme/default_200_percent/chromium/product_logo_32.png
./chrome/app/theme/default_200_percent/chromium/product_logo_white.png
./chrome/app/theme/default_100_percent/chromium/product_logo_16.png
./chrome/app/theme/default_100_percent/chromium/product_logo_name_22.png
./chrome/app/theme/default_100_percent/chromium/product_logo_name_48.png
./chrome/app/theme/default_100_percent/chromium/product_logo_32.png
./chrome/app/theme/default_100_percent/chromium/product_logo_white.png

If you can give me input on how the grd files (app/theme/brave_theme_resources.grd) work regarding the icons I'd appreciate it.

@simonhong
Copy link
Member

FYI, windows icons in linux are handled by below in brave_views_delegate_linux.cc.

int GetWindowIconResourceId() {
#if defined(OFFICIAL_BUILD)
  switch (chrome::GetChannel()) {
    case version_info::Channel::DEV:
      return IDR_PRODUCT_LOGO_128_DEV;
    case version_info::Channel::BETA:
      return IDR_PRODUCT_LOGO_128_BETA;
    case version_info::Channel::CANARY:
      return IDR_PRODUCT_LOGO_128_NIGHTLY;
    default:
      return IDR_PRODUCT_LOGO_128;
  }
#else
  return IDR_PRODUCT_LOGO_128_DEVELOPMENT;
#endif
}

The resources are defined in brave_unscaled_resources.grd.
However, I assume that icon in package file uses different method(not sure).
I'll also try to find how icon is involved in packaging.

@simonhong
Copy link
Member

I think we should add more resources like is_chrome_branded

copy("theme_files") {
  visibility = [ ":*" ]
  sources = [
    "$branding_dir/BRANDING",
    "$branding_dir/linux/product_logo_32.xpm",
    "$branding_dir/product_logo_128.png",
    "$branding_dir/product_logo_22.png",
    "$branding_dir/product_logo_24.png",
    "$branding_dir/product_logo_256.png",
    "$branding_dir/product_logo_48.png",
    "$branding_dir/product_logo_64.png",
    "$branding_dir_100/product_logo_16.png",
    "$branding_dir_100/product_logo_32.png",
  ]
  if (is_chrome_branded) {
    sources += [
      "$branding_dir/linux/product_logo_32_beta.xpm",
      "$branding_dir/linux/product_logo_32_dev.xpm",
      "$branding_dir/product_logo_128_beta.png",
      "$branding_dir/product_logo_128_dev.png",
      "$branding_dir/product_logo_22_beta.png",
      "$branding_dir/product_logo_22_dev.png",
      "$branding_dir/product_logo_24_beta.png",
      "$branding_dir/product_logo_24_dev.png",
      "$branding_dir/product_logo_256_beta.png",
      "$branding_dir/product_logo_256_dev.png",
      "$branding_dir/product_logo_48_beta.png",
      "$branding_dir/product_logo_48_dev.png",
      "$branding_dir/product_logo_64_beta.png",
      "$branding_dir/product_logo_64_dev.png",
      "$branding_dir_100/product_logo_16_beta.png",
      "$branding_dir_100/product_logo_16_dev.png",
      "$branding_dir_100/product_logo_32_beta.png",
      "$branding_dir_100/product_logo_32_dev.png",
    ]
  }
  outputs = [
    "$root_out_dir/installer/theme/{{source_file_part}}",
  ]
}

In chromium project, I think chrome team does not include google_chrome brand resources in chrome/app/theme/google_chrome folder. Because of that, I think we can't see above files in is_chrome_branded.

I think we should add more resources for linux like is_chrome_branded did.

@srirambv
Copy link
Contributor Author

this is also showing shown on app launcher and here

linux

@mbacchi
Copy link
Contributor

mbacchi commented Aug 28, 2018

Thanks for that input @simonhong. Since we only have one dev/beta branded png image, I will configure the build to include that for now, but we should open another issue to create the full complement of dev/beta images. There are a number of uses for the various sizes of png images, and without all of them we are not providing the full experience that chrome has.

If I change the build scripts to include both the standard png images and the dev/beta png images, the dev channel build behavior still shows as above, with the normal color product logo when launching the browser. If I instead only include the dev/beta product logo it properly shows the blue dev channel product logo. But this is the only logo included in that dev/beta build and I would expect we need other logo sizes for other images.

For now this single image works in my testing.

mbacchi added a commit to brave/brave-core that referenced this issue Aug 28, 2018
When building dev/beta channel on Linux, the release
png images were being packaged even though the
alternate color images should have been present in
the package. This corrects that behavior.

Fixes brave/brave-browser#712
mbacchi added a commit to brave/brave-core that referenced this issue Aug 29, 2018
When building dev/beta channel on Linux, the release
png images were being packaged even though the
alternate color images should have been present in
the package. This corrects that behavior.

Fixes brave/brave-browser#712
@bbondy bbondy added the QA/Yes label Sep 10, 2018
@srirambv
Copy link
Contributor Author

Verification Passed on

Brave 0.55.5 Chromium: 70.0.3538.16 (Official Build) dev (64-bit)
Revision 16ed95b41bb05e565b11fb66ac33c660b721f778-refs/branch-heads/3538@{#306}
OS Linux

image

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