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

chrome.VisualElementsManifest.xml is not renamed on Windows #5410

Open
Iffabled opened this issue Jul 26, 2019 · 4 comments
Open

chrome.VisualElementsManifest.xml is not renamed on Windows #5410

Iffabled opened this issue Jul 26, 2019 · 4 comments

Comments

@Iffabled
Copy link

@Iffabled Iffabled commented Jul 26, 2019

Test plan

See brave/brave-core#4299

Description

Windows 10 allows you to create a <app_name>.VisualElementsManifest.xml file for your <app_name>.exe, that configures app's look in the Start Menu. Chrome comes with it by default, and it is bundled with Brave, but it is not renamed to brave.VisualElementsManifest.xml, so the OS doesn't detect it.

Steps to Reproduce

  1. Open the Start Menu.
  2. Pin Brave to Start.
  3. Look at Brave's entry on the app list and at Brave's tile.

Actual result:

Brave's tile and app list entry are not customized.

Brave's small tile with the file named incorrectly

Expected result:

Brave's tile and app list entry are customized.

Brave's small tile with the file named correctly

Reproduces how often:

Always reproduces.

Brave version (brave://version info)

Brave | 0.66.101 Chromium: 75.0.3770.142 (Official Build) (64-bit)
Revision | 8d93477bd18154c67a70a67019cb8032382c55dd-refs/branch-heads/3770@{#1160}
OS | Windows 10 OS Version 1903 (Build 18362.239)

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? No
  • Does the issue resolve itself when disabling Brave Rewards? No
  • Is the issue reproducible on the latest version of Chrome? No

Miscellaneous Information:

This file can be used to customize things like the tile's background color, icons used for the smaller and the medium tile, whether the tile has the app's name or what color is the app's name.

@rebron rebron added the priority/P4 label Aug 9, 2019
@rebron rebron added this to P4 Backlog in General Aug 14, 2019
@rossjrw
Copy link

@rossjrw rossjrw commented Dec 20, 2019

Noting that the actual result listed here is not what I experienced - on my end, the custom colour background shows up as the system colour, not as Brave's grey.

Not really sure why it's grey when the secondary colour everywhere else is purple.

@Iffabled
Copy link
Author

@Iffabled Iffabled commented Dec 22, 2019

Noting that the actual result listed here is not what I experienced - on my end, the custom colour background shows up as the system colour, not as Brave's grey.

I have set my system color to grey.

Not really sure why it's grey when the secondary colour everywhere else is purple.

I didn't modify the file's content to better suit Brave, I just renamed it so that it would be loaded by Windows.

@bsclifton bsclifton self-assigned this Dec 31, 2019
bsclifton added a commit to brave/brave-core that referenced this issue Jan 2, 2020
bsclifton added a commit to brave/brave-core that referenced this issue Jan 2, 2020
@bsclifton bsclifton added this to the 1.5.x - Nightly milestone Jan 3, 2020
bsclifton added a commit to brave/brave-core that referenced this issue Jan 3, 2020
bsclifton added a commit to brave/brave-core that referenced this issue Jan 6, 2020
bsclifton added a commit to brave/brave-core that referenced this issue Jan 8, 2020
bsclifton added a commit to brave/brave-core that referenced this issue Jan 8, 2020
bsclifton added a commit to brave/brave-core that referenced this issue Jan 9, 2020
General automation moved this from P4 Backlog to Completed Jan 10, 2020
@bsclifton
Copy link
Member

@bsclifton bsclifton commented Jan 10, 2020

I did an implementation of this but only after merging did I realize we aren't creating different versions per release channel

Implementation I did:
brave/brave-core#4299

Reverted with:
brave/brave-core#4358

@bsclifton bsclifton reopened this Jan 10, 2020
@bsclifton
Copy link
Member

@bsclifton bsclifton commented Jan 10, 2020

When solving this issue properly (respecting channels), we can update the graphics used for the icons if we'd like:
https://github.com/brave/brave-core/tree/master/app/theme/brave/win/tiles

  • Logo.png will be scaled (if needed) and displayed at 150px by 150px
  • SmallLogo.png will be scaled (if needed) and displayed at 70px by 70px

We can also change the background color (default is gray) and we can turn on/configure text color

The proper thing to do would be to add the icons under https://github.com/brave/brave-core/tree/master/app/theme/brave/win and then copy the channel specific ones into the tiles directory at build time via GN

It's possible this is already supported by the installer; for more info see:
https://cs.chromium.org/chromium/src/chrome/BUILD.gn?l=1511-1516&rcl=d08e75a3f95509e23b3793fb8ad0027dc8f4a2f7

Here's what the icons look like after wiring up brave.VisualElementManifest.xml:
(actual tile circled in RED)

Medium tile

Screen Shot 2020-01-10 at 12 00 23 AM

Small tile

Screen Shot 2020-01-10 at 12 00 51 AM

@bsclifton bsclifton removed their assignment Jan 10, 2020
@bsclifton bsclifton removed this from the 1.5.x - Nightly milestone Jan 10, 2020
@bsclifton bsclifton moved this from Completed to In progress in General Jan 10, 2020
@bsclifton bsclifton moved this from In progress to P4 Backlog in General Jan 10, 2020
General automation moved this from P4 Backlog to Completed Jan 16, 2020
@bsclifton bsclifton reopened this Jan 20, 2020
@bsclifton bsclifton moved this from Completed to Pending review in General Jan 20, 2020
@bsclifton bsclifton moved this from Pending review to P4 Backlog in General Jan 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
General
  
P4 Backlog
Linked pull requests

Successfully merging a pull request may close this issue.

5 participants
You can’t perform that action at this time.