Add new keyboard shortcuts for things not already customizable

[hamirmahal]

Platforms

Linux, macOS, Windows

Description

It'd be nice if users could create custom keyboard shortcuts to do specific things in Brave.

[hamirmahal]

For example, it'd be useful if I could do Ctrl + Shift + E to export all bookmarks when on the bookmarks page.

[hamirmahal]

It'd also be nice if I could add a shortcut to clear all downloads, maybe with Ctrl + Shift + C, when on the downloads page.

[hamirmahal]

I think it'd be really helpful if user-created custom keyboard shortcuts could be page-specific.

[bsclifton]

@hamirmahal we have brave://settings/system/shortcuts - are there things we're missing? I guess check the page and see if the above actions are registered on the page

cc: @fallaciousreasoning

[hamirmahal]

@bsclifton it looks like there aren't any preexisting shortcuts for clearing all downloads and exporting bookmarks.

I think it'd be really useful for users if we could add custom keyboard shortcuts for arbitrary browser actions.

[fallaciousreasoning]

Hey @hamirmahal! Thanks, for you feedback - I agree that it'd be pretty handy, but unfortunately it will probably be quite a bit of work to expose everything the browser can do as a command :face_with_diagonal_mouth:

If you wanted to add a command for exporting bookmarks and/or clearing downloads we'd definitely accept a PR - otherwise it's probably going to be a while until we can get to it though.

[hamirmahal]

Hello, @fallaciousreasoning! You're welcome.

I'm interested in what the process looks like for adding a new keyboard shortcut. What parts of the code base would someone need to modify to do that?

[fallaciousreasoning]

This PR here adds a few new commands: https://github.com/brave/brave-core/pull/19035

Basically there are a few different bits:

1. Add a new command to app/brave_command_ids.h (normally at the bottom with a little gap with the other ids)
2. Add case statements for the new command in browser/ui/brave_browser_command_controller.cc
3. Implement the functionality in browser/ui/browser_commands.cc (probably the hardest part will be finding out where the existing code to Clear history / export bookmarks lives, and how to call it)
4. Add a string for the command as IDS_IDC_<COMMAND_NAME> in commands.grdp

After this the commands should be available from the command palette and brave://settings/system/shortcuts

5. Optional: If you want to add a default shortcut, add an entry in chromium_src/chrome/browser/ui/views/accelerator_table.cc

The hardest parts will probably be:

1. Getting Brave initially building locally
2. Finding the code which you want your command to trigger

Happy to lend a hand if you need it!

[hamirmahal]

Thanks for offering to help!

I did some rebasing after making some changes locally and some commands like npm run sync don't work anymore.

I don't mind re-cloning the repository for a fresh slate, but I'd like to avoid re-downloading 30 GB for chromium. Are there any docs you can point me to for fixing sync issues?

[hamirmahal]

https://github.com/brave/brave-browser/issues/40509

[fallaciousreasoning]

Hmm, that's weird, normally sync is pretty consistent. (completely understand not wanting to reclone Chromium - its massive)!

Can you try:

npm run init -- --force -C -D

followed by an

npm run sync

[hamirmahal]

Hmm... I should be running npm run init -- --force -C -D in brave-browser/src/brave, right?

1. I did git pull in brave-browser/.
2. cd src/brave/
3. git pull
4. npm run init -- --force -C -D

It errors with a similar message. The working tree was clean in brave-browser/src/brave and brave-browser before doing npm run init -- --force -C -D, although src/.gitkeep is gone from brave-browser/ afterward.

[Error: ENOENT: no such file or directory, open '/home/hamir/brave-browser/src/base/version_info/BUILD.gn'] {
  errno: -2,
  code: 'ENOENT',
  syscall: 'open',
  path: '/home/hamir/brave-browser/src/base/version_info/BUILD.gn'
}

Full output

``` ~/.../src/brave (master)$ npm run init -- --force -C -D > brave-core@1.71.8 init > cd ../../ && npm run --prefix src/brave sync -- --init --force -C -D > brave-core@1.71.8 sync > node ./build/commands/scripts/sync.js --init --force -C -D gclient sync... -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- /home/hamir/brave-browser/src > git rev-parse HEAD -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- /home/hamir/brave-browser/src > git rev-parse refs/tags/128.0.6613.40 -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- /home/hamir/brave-browser/src > git log -n 1 --pretty=format:%h%d Chromium repo needs sync. target is refs/tags/128.0.6613.40 at commit ce7f05019fe091c912aa09c989eb3d7d406d0f3c current commit is 4e22a93b12f76 (HEAD -> main, origin/main, origin/HEAD) at commit 4e22a93b12f76cc59ee084f7f6887afc232e2afd latest successful sync is { "chromiumRef": "refs/tags/128.0.6613.40", "gClientTimestamp": "1722654117101.6826" } -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- /home/hamir/brave-browser > gclient sync --nohooks --revision src@refs/tags/128.0.6613.40 --reset --upstream --force -D Syncing projects: 100% (166/166), done. -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- /home/hamir/brave-browser/src > git log -n 1 --pretty=format:%h%d Chromium is now at 4e22a93b12f76 (HEAD -> main, origin/main, origin/HEAD) -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- /home/hamir/brave-browser/src/brave > gclient sync --nohooks --force -D Syncing projects: 100% (14/14), done. ...gclient sync done -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- /home/hamir/brave-browser/src > vpython3 /home/hamir/brave-browser/src/brave/vendor/depot_tools/build_telemetry.py opt-out apply patches... Brave Browser Sync ERROR: [Error: ENOENT: no such file or directory, open '/home/hamir/brave-browser/src/base/version_info/BUILD.gn'] { errno: -2, code: 'ENOENT', syscall: 'open', path: '/home/hamir/brave-browser/src/base/version_info/BUILD.gn' } 1 ~/.../src/brave (master)$ git status On branch master Your branch is up to date with 'origin/master'. nothing to commit, working tree clean 0 ~/.../src/brave (master)$ git pull Already up to date. ```

[fallaciousreasoning]

Yeah that sounds right @hamirmahal.

Hmm, could you try manually resetting Chromium (in your brave-browser/src folder git reset --hard ce7f05019fe091c912aa09c989eb3d7d406d0f3c)

And in your brave folder

git fetch
git rebase --onto origin/master

and then try to init again?

[fallaciousreasoning]

Ah, the git pull should be in brave-browser/src/brave - we pretty much soley work in the brave-browser/src/brave directory :smile:

[hamirmahal]

Hmm... That doesn't seem to work, either.

Full output

``` 0 ~/brave-browser/src (main)$ git reset --hard ce7f05019fe091c912aa09c989eb3d7d406d0f3c Updating files: 100% (226131/226131), done. HEAD is now at ce7f05019fe09 Incrementing VERSION to 128.0.6613.40 0 ~/brave-browser/src (main)$ cd brave/ 0 ~/.../src/brave (master)$ git fetch 0 ~/.../src/brave (master)$ git rebase --onto origin/mastegit rebase --onto origin/master Current branch master is up to date. 0 ~/.../src/brave (master)$ npm run init -- --force -C -D > brave-core@1.71.23 init > cd ../../ && npm run --prefix src/brave sync -- --init --force -C -D > brave-core@1.71.23 sync > node ./build/commands/scripts/sync.js --init --force -C -D gclient sync... -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- /home/hamir/brave-browser/src > git rev-parse HEAD -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- /home/hamir/brave-browser/src > git rev-parse refs/tags/128.0.6613.40 Chromium repo does not need sync as it is already refs/tags/128.0.6613.40 at commit ce7f05019fe091c912aa09c989eb3d7d406d0f3c. -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- /home/hamir/brave-browser > gclient sync --nohooks --revision src@refs/tags/128.0.6613.40 --reset --upstream --force -D Syncing projects: 100% (166/166), done. -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- /home/hamir/brave-browser/src > git log -n 1 --pretty=format:%h%d Chromium is now at 4e22a93b12f76 (HEAD -> main, origin/main, origin/HEAD) -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- /home/hamir/brave-browser/src/brave > gclient sync --nohooks --force -D Syncing projects: 100% (14/14), done. ...gclient sync done -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- /home/hamir/brave-browser/src > vpython3 /home/hamir/brave-browser/src/brave/vendor/depot_tools/build_telemetry.py opt-out apply patches... Brave Browser Sync ERROR: [Error: ENOENT: no such file or directory, open '/home/hamir/brave-browser/src/base/version_info/BUILD.gn'] { errno: -2, code: 'ENOENT', syscall: 'open', path: '/home/hamir/brave-browser/src/base/version_info/BUILD.gn' } 1 ~/.../src/brave (master)$ ```

[fallaciousreasoning]

okay, that seems super weird - it seems to think you're on the right revision but the version_info/BUILD.gn file doesn't exist.

If you check, does that folder/file exist? I checked on my local and its there. Interestingly it looks like it might be failing to apply patches.

Do you get the same error running

npm run apply_patches

(in src/brave)

[hamirmahal]

If you check, does that folder/file exist? I checked on my local and its there. Interestingly it looks like it might be failing to apply patches.

It looks like that file doesn't exist.

--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
/home/hamir/brave-browser/src
> vpython3 /home/hamir/brave-browser/src/brave/vendor/depot_tools/build_telemetry.py opt-out
apply patches...
Brave Browser Sync ERROR:
[Error: ENOENT: no such file or directory, open '/home/hamir/brave-browser/src/base/version_info/BUILD.gn'] {
  errno: -2,
  code: 'ENOENT',
  syscall: 'open',
  path: '/home/hamir/brave-browser/src/base/version_info/BUILD.gn'
}

1 ~/.../src/brave (master)$  cd ..
0 ~/brave-browser/src (main)$  ls 
android_webview/         chrome/                  dbus/                    .gitignore               .mailmap                 PRESUBMIT_test.py        testing/
apps/                    chromecast/              DEPS                     .gn                      media/                   printing/                third_party/
ash/                     chromeos/                device/                  google_apis/             mojo/                    README.md                tools/
ATL_OWNERS               .clang-format            DIR_METADATA             google_update/           native_client/           remoting/                ui/
AUTHORS                  .clang-tidy              docs/                    gpu/                     native_client_sdk/       rlz/                     url/
base/                    codelabs/                .eslintrc.js             headless/                net/                     .rustfmt.toml            v8/
brave/                   CODE_OF_CONDUCT.md       extensions/              infra/                   out/                     sandbox/                 .vpython3
build/                   codereview.settings      fuchsia_web/             ios/                     OWNERS                   services/                WATCHLISTS
BUILD.gn                 components/              gin/                     ipc/                     pdf/                     skia/                    weblayer/
build_overrides/         content/                 .git/                    .landmines               ppapi/                   sql/                     .yapfignore
buildtools/              courgette/               .gitattributes           LICENSE                  PRESUBMIT.py             storage/                 
cc/                      crypto/                  .git-blame-ignore-revs   LICENSE.chromium_os      PRESUBMIT_test_mocks.py  styleguide/              
0 ~/brave-browser/src (main)$  ls base/version_info
ls: cannot access 'base/version_info': No such file or directory
2 ~/brave-browser/src (main)$  ls base/version
version.cc           version.h            version_unittest.cc

And I get a similar error when doing npm run apply_patches.

~/.../src/brave (master)$  npm run apply_patches

> brave-core@1.71.23 apply_patches
> node ./build/commands/scripts/commands.js apply_patches

apply patches...
[Error: ENOENT: no such file or directory, open '/home/hamir/brave-browser/src/base/version_info/BUILD.gn'] {
  errno: -2,
  code: 'ENOENT',
  syscall: 'open',
  path: '/home/hamir/brave-browser/src/base/version_info/BUILD.gn'
}

[fallaciousreasoning]

Okay that's interesting - it sounds like your Chromium checkout might be in a weird state. I just had a look at the base/version_info/BUILD.gn and it looks like its been around since 2015 at least.

My Chromium is at 5b672c5c04334afc4a7d0a025171853adedb799d as of today. If you do a git show --stat in the src directory, what does it show?

Additionally, what is the Chromium version you have in src/brave/package.json (mine says "tag": "128.0.6613.85")

One last debugging idea - what if you do a git status | grep version_info in src? Mine shows base/version_info/BUILD.gn as modified - maybe yours has somehow been removed (in which case, maybe a git checkout -- base/version_info/BUILD.gn might help).

I wonder how it got into this state? Normally sync is pretty good at making sure brave/chrome are pointing at the right version

[hamirmahal]

git show --stat

``` ~/brave-browser/src (main)$ git show --stat commit 4e22a93b12f76cc59ee084f7f6887afc232e2afd (HEAD -> main, origin/main, origin/HEAD) Author: Takashi Sakamoto Date: Fri Jul 28 08:27:42 2023 +0000 [PA] Replace PA_[D]CHECK() under partition_alloc_base with PA_BASE_[D]CHECK(). Make //base/allocator/partition_allocator/partition_alloc_base not depend on //base/allocator/partition_allocator. Change-Id: I314736d5613d891ddf74ee7e6720e7871dc10610 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4720679 Reviewed-by: Yuki Shiino Commit-Queue: Takashi Sakamoto Cr-Commit-Position: refs/heads/main@{#1176487} base/allocator/partition_allocator/partition_alloc_base/bits.h | 6 +++--- base/allocator/partition_allocator/partition_alloc_base/cxx17_backports.h | 4 ++-- base/allocator/partition_allocator/partition_alloc_base/files/file_path.cc | 4 ++-- base/allocator/partition_allocator/partition_alloc_base/mac/foundation_util.mm | 4 ++-- base/allocator/partition_allocator/partition_alloc_base/mac/mac_util.mm | 4 ++-- base/allocator/partition_allocator/partition_alloc_base/mac/scoped_typeref.h | 4 ++-- base/allocator/partition_allocator/partition_alloc_base/memory/ref_counted.cc | 4 ++-- base/allocator/partition_allocator/partition_alloc_base/memory/ref_counted.h | 18 +++++++++--------- base/allocator/partition_allocator/partition_alloc_base/memory/scoped_refptr.h | 10 +++++----- base/allocator/partition_allocator/partition_alloc_base/native_library_posix.cc | 4 ++-- base/allocator/partition_allocator/partition_alloc_base/rand_util.cc | 4 ++-- base/allocator/partition_allocator/partition_alloc_base/rand_util_pa_unittest.cc | 5 +++-- base/allocator/partition_allocator/partition_alloc_base/rand_util_posix.cc | 6 +++--- base/allocator/partition_allocator/partition_alloc_base/rand_util_win.cc | 8 ++++---- base/allocator/partition_allocator/partition_alloc_base/threading/platform_thread_mac_for_testing.mm | 4 ++-- base/allocator/partition_allocator/partition_alloc_base/threading/platform_thread_posix_for_testing.cc | 6 +++--- base/allocator/partition_allocator/partition_alloc_base/threading/platform_thread_win_for_testing.cc | 10 +++++----- base/allocator/partition_allocator/partition_alloc_base/time/time.h | 14 +++++++------- base/allocator/partition_allocator/partition_alloc_base/time/time_conversion_posix.cc | 6 +++--- base/allocator/partition_allocator/partition_alloc_base/time/time_fuchsia.cc | 8 ++++---- base/allocator/partition_allocator/partition_alloc_base/time/time_mac.mm | 20 ++++++++++---------- base/allocator/partition_allocator/partition_alloc_base/time/time_now_posix.cc | 6 +++--- base/allocator/partition_allocator/partition_alloc_base/time/time_override.cc | 4 ++-- base/allocator/partition_allocator/partition_alloc_base/time/time_win.cc | 17 +++++++++-------- 24 files changed, 91 insertions(+), 89 deletions(-) ```

---

src/brave/package.json

  "config": {
    "projects": {
      "chrome": {
        "dir": "src",
        "tag": "128.0.6613.40",
        "repository": {
          "url": "https://github.com/brave/chromium"
        }
      },
      "depot_tools": {
        "dir": "vendor/depot_tools",
        "repository": {
          "url": "https://chromium.googlesource.com/chromium/tools/depot_tools.git"
        }
      }
    }
  },

---

0 ~/brave-browser/src (main)$  git status | grep version_info
1 ~/brave-browser/src (main)$ 

---

1 ~/brave-browser/src (main)$  git checkout -- base/version_info/BUILD.gn
error: pathspec 'base/version_info/BUILD.gn' did not match any file(s) known to git
1 ~/brave-browser/src (main)$ 

---

Just as a sanity check, are the remote URLs for each of these directories correct?

0 ~/brave-browser (master)$ git remote get-url origin
https://github.com/brave/brave-browser/
0 ~/brave-browser (master)$ 

0 ~/brave-browser/src (main)$ git remote get-url origin
https://github.com/brave/chromium
0 ~/brave-browser/src (main)$ 

0 ~/.../src/brave (master)$ git remote get-url origin
https://github.com/brave/brave-core
0 ~/.../src/brave (master)$ 

[fallaciousreasoning]

@hamirmahal I had a talk to a few other people and it sounds like maybe something went wrong with the initial checkout? Not sure what to do at this point except try recloning.

(the remote URLs all look good to me)