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

(Win/Linux) New Private window should be New private window, in hamburger menu #17377

Closed
stephendonner opened this issue Aug 5, 2021 · 20 comments · Fixed by brave/brave-core#17387
Assignees
Labels

Comments

@stephendonner
Copy link

stephendonner commented Aug 5, 2021

Implementation note (from @mkarolin; updated by @bsclifton)

  • The _override.grd files can't be modified; these are auto-generated when we run npm run chromium_rebase_l10n
  • The strings in the _override.grd files are based on automatic generic substitutions that are mapped in https://github.com/brave/brave-core/blob/master/script/lib/grd_string_replacements.py
  • There is no mapping for Incognito -> private
  • One option to solve this is to add a new string into app/brave_generated_resources.grd and then replace IDS_NEW_INCOGNITO_WINDOW with our string ID in code via a chromium_src override.

Original issue description

New Private window should be New private window, in hamburger menu

Windows and Linux desktop only;macOS is taken care of in #17308

Steps to Reproduce

  • open the hamburger menu on the browser toolbar
  • look at the capitalization of the menu items

Actual result:

Reads New Private window

windows10-menu

Expected result:

New private window

Reproduces how often:

100%

Brave version (brave://version info)

Brave 1.30.3 Chromium: 92.0.4515.131 (Official Build) nightly (64-bit)
Revision 6b8d6c56ce21e38a72f7c4becb5abc1fa5134f29-refs/branch-heads/4515@{#1933}
OS Windows 10 OS Version 2009 (Build 22000.120)

/cc @nullhook

@rebron
Copy link
Collaborator

rebron commented Aug 6, 2021

@stephendonner Windows and Linux?

@rebron rebron added the priority/P4 Planned work. We expect to get to it "soon". label Aug 6, 2021
@stephendonner
Copy link
Author

@stephendonner Windows and Linux?

Yup.

@stephendonner stephendonner changed the title New Private window should be New private window, in hamburger menu (Win/Linux) New Private window should be New private window, in hamburger menu Aug 6, 2021
@SebastianLF
Copy link

Let's give it a try.

@stephendonner
Copy link
Author

Let's give it a try.

Hi @SebastianLF by this, do you mean to say you'd like to try fixing this issue? If so, please confirm, and I'll assign to you; thanks!

@SebastianLF
Copy link

SebastianLF commented Aug 8, 2021

Let's give it a try.

Hi @SebastianLF by this, do you mean to say you'd like to try fixing this issue? If so, please confirm, and I'll assign to you; thanks!

Exactly. I will work on it tomorrow.

@SebastianLF
Copy link

Package 'libgnome-keyring-dev' is no more maintained on my actual version of ubuntu 20.04. I'll install version 18.04 in a new partition on my disk and make it work.

@stephendonner
Copy link
Author

How's this going @SebastianLF? If for some reason you're (later) unable to make progress, please unassign it back, so others can also take a look; thanks!

@SebastianLF
Copy link

SebastianLF commented Aug 12, 2021

@stephendonner I installed ubuntu 18.04 and set up everything today. I certainly send a PR tomorrow.

@SebastianLF
Copy link

when "npm run init", it starts to download 16gb of data, why is that big ?

@bsclifton
Copy link
Member

bsclifton commented Aug 13, 2021

@SebastianLF running npm run init will download all of the source from Chromium (including history, which is the biggest). If you're able to compile locally, that's great. But you can still PR the text change without trying locally and we can try on our side with our CI (or building locally)

@SebastianLF
Copy link

@SebastianLF running npm run init will download all of the source from Chromium (including history, which is the biggest). If you're able to compile locally, that's great. But you can still PR the text change without trying locally and we can try on our side with our CI (or building locally)

Thanks for your answer, I'll do this way. Is it in 'generated_resources_override.grd' or 'generated_resources.grd' that the string should be modified?
Sorry guys for the delay.

@bsclifton
Copy link
Member

bsclifton commented Aug 17, 2021

@SebastianLF we have some info on our wiki here:
https://github.com/brave/brave-browser/wiki/Strings-and-Localization#chromium-strings

But I'm not seeing that addressed unfortunately. @mkarolin @petemill @mariospr do we have any docs for when to use app/generated_resources.grd versus app/generated_resources_override.grd?

@deepxcode
Copy link

I can work on this issue and make the PR asap.

@AnudeepGunukula
Copy link

Hi @bsclifton @stephendonner . i had made a pr that will close this issue
kindly review it and let me know the changes if needed.

@stephendonner
Copy link
Author

Folks, thanks for your willingness to help; @SebastianLF still has this assigned to him, so I'd like to give him first attempt at solving it; thanks!

@SebastianLF
Copy link

SebastianLF commented Aug 23, 2021

PR: brave/brave-core#9821
Let me know if it works. If any error, I will modify accordingly.

@stephendonner
Copy link
Author

PR: brave/brave-core#9821
Let me know if it works. If any error, I will modify accordingly.

Sorry for the delay; I've put a couple more reviewers on your PR, who will hopefully be able to take a look soon! 🙏

@bsclifton
Copy link
Member

bsclifton commented Sep 2, 2021

Thanks for the attempt @SebastianLF - I removed the good first issue tag as there is some complexity here due to the replacement done for Chromium strings by Brave. More information available in brave/brave-core#9821 (comment)

Updated the top issue with a note

@rebron rebron added priority/P3 The next thing for us to work on. It'll ride the trains. and removed priority/P4 Planned work. We expect to get to it "soon". labels Feb 24, 2023
@spylogsster spylogsster self-assigned this Feb 27, 2023
@brave-builds brave-builds added this to the 1.50.x - Nightly milestone Feb 27, 2023
@stephendonner
Copy link
Author

Verified PASSED using

Brave 1.50.87 Chromium: 111.0.5563.64 (Official Build) dev (64-bit)
Revision c710e93d5b63b7095afe8c2c17df34408078439d-refs/branch-heads/5563@{#995}
OS Linux

Confirmed string now reads New private window with Tor

Screen Shot 2023-03-11 at 11 12 03 PM

@stephendonner
Copy link
Author

Verified PASSED using

Brave 1.50.87 Chromium: 111.0.5563.64 (Official Build) dev (64-bit)
Revision c710e93d5b63b7095afe8c2c17df34408078439d-refs/branch-heads/5563@{#995}
OS Windows 10 Version 22H2 (Build 19045.2673)

Confirmed string now reads New private window with Tor

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
9 participants