feat: add --disable-geolocation command-line flag for macOS by nilayarya · Pull Request #45934 · electron/electron

nilayarya · 2025-03-09T15:11:49Z

Description of Change

Added a command line flag to opt-out of location services on macOS and prevent apps from listing themselves in location services.

Fixes: #32606

Checklist

PR description included and stakeholders cc'd
npm test passes
tests are changed or added
relevant API documentation, tutorials, and examples are updated and follow the documentation style guide
PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

Notes: Added --disable-geolocation command-line flag for macOS apps to disable location services

nilayarya · 2025-03-09T15:14:00Z

@deepak1556 Is this platform specific command-line flag fine or does it have to be extended to other platforms?

nilayarya · 2025-03-14T10:12:05Z

Hi @codebytere, could I get a review on this?

deepak1556

The PR is still set to draft hence reviews aren't appearing. Can you set it to ready so that wg-api can chime in.

deepak1556 · 2025-03-14T17:14:01Z

docs/api/command-line-switches.md


+### --disable-geolocation-mac
+
+Disables the Geolocation API on macOS. Has no effect on other platforms.


If an application sets this flag and accidentally calls navigator.geolocation api would that lead to a crash ?

If this flag is set should we deny permission for geolocation via https://github.com/electron/electron/blob/main/docs/api/session.md#sessetpermissionrequesthandlerhandler internally so that calls are canceled before they reach the device service ?

@deepak1556 the app doesn't crash on my mac despite setting this flag and calling navigator.geolocation. But this could be due to the api not working correctly on Mac and as a result calls never reaching device service. Some concerns about this have been raised in the past Issue#45290

I’ve added some changes to deny permission for geolocation via https://github.com/electron/electron/blob/main/docs/api/session.md#sessetpermissionrequesthandlerhandler internally so that calls are canceled before they reach the device service. I need a review on this. Thanks.

Can we document how this impacts the permission request API?

Disables the Geolocation API. Permission requests for geolocation will be denied internally regardless of the decision made by a handler set via session.setPermissionRequestHandler. This functionality is currently implemented only for macOS. Has no effect on other platforms.

Is this okay?

That sounds great. Thanks!

deepak1556 · 2025-03-14T17:15:26Z

docs/api/command-line-switches.md


 Disable HTTP/2 and SPDY/3.1 protocols.

+### --disable-geolocation-mac


This can be a generic flag --disable-geolocation and document it only for __macOS__ now which can be extended to other platforms if system services are integrated. I see wip in upstream for something similar on windows.

shell/browser/api/electron_api_session.cc

samuelmaddock

API LGTM

docs/api/command-line-switches.md

shell/browser/api/electron_api_session.cc

itsananderson · 2025-04-10T17:24:46Z

docs/api/command-line-switches.md


+### --disable-geolocation-mac
+
+Disables the Geolocation API on macOS. Has no effect on other platforms.


Can we document how this impacts the permission request API?

itsananderson

Sorry for my slow response 😅

itsananderson · 2025-04-17T17:34:51Z

docs/api/command-line-switches.md


+### --disable-geolocation-mac
+
+Disables the Geolocation API on macOS. Has no effect on other platforms.


That sounds great. Thanks!

shell/browser/api/electron_api_session.cc

nilayarya · 2025-04-19T17:19:21Z

Sorry for my slow response 😅

No rush at all. You can check out the latest changes at your earliest convenience. Thanks!

itsananderson

Left a small suggestion but otherwise API LGTM

docs/api/command-line-switches.md

nilayarya · 2025-04-29T21:17:22Z

Hey @itsananderson I've added some tests and rebased. Lmk if there's anything else that needs to be done in order to land this pr. Thanks!

deepak1556

LGTM, a couple more minor style changes and should be good to land. Thanks!

shell/browser/electron_permission_manager.h

spec/chromium-spec.ts

…ager as a static member

Co-authored-by: Robo <hop2deep@gmail.com>

nilayarya · 2025-05-04T23:09:10Z

@itsananderson can we restart the CI checks pls

itsananderson · 2025-05-08T22:03:33Z

The test builds seem like they might be having actual failures:

not ok 1430 WebContentsView setBorderRadius capture should render with cutout corners
  Expected color at (10, 10) to match '#0000ff', but got '#dadada'. See the artifact 'color-mismatch-31ad33d2399f94a59f546189.png' for more information.
  AssertionError: Expected color at (10, 10) to match '#0000ff', but got '#dadada'. See the artifact 'color-mismatch-31ad33d2399f94a59f546189.png' for more information.
      at ScreenCapture._expectImpl (electron/spec/lib/screen-helpers.ts:192:13)
      at Context.<anonymous> (electron/spec/api-web-contents-view-spec.ts:325:11)
not ok 1431 WebContentsView setBorderRadius capture should allow resetting corners
  Expected color at (10, 10) to match '#00b140', but got '#dadada'. See the artifact 'color-mismatch-eeda5c6639338c70c5141682.png' for more information.
  AssertionError: Expected color at (10, 10) to match '#00b140', but got '#dadada'. See the artifact 'color-mismatch-eeda5c6639338c70c5141682.png' for more information.
      at ScreenCapture._expectImpl (electron/spec/lib/screen-helpers.ts:192:13)
      at Context.<anonymous> (electron/spec/api-web-contents-view-spec.ts:338:9)
not ok 1432 WebContentsView setBorderRadius capture should render when set before attached
  Expected color at (10, 10) to match '#0000ff', but got '#dadada'. See the artifact 'color-mismatch-9b9748371c36282ee5463a1b.png' for more information.
  AssertionError: Expected color at (10, 10) to match '#0000ff', but got '#dadada'. See the artifact 'color-mismatch-9b9748371c36282ee5463a1b.png' for more information.
      at ScreenCapture._expectImpl (electron/spec/lib/screen-helpers.ts:192:13)
      at Context.<anonymous> (electron/spec/api-web-contents-view-spec.ts:354:9)

That said, I don't know why this PR would be causing those tests to fail. Maybe they're just flaky?

nilayarya · 2025-05-08T22:28:51Z

The test builds seem like they might be having actual failures:


not ok 1430 WebContentsView setBorderRadius capture should render with cutout corners

  Expected color at (10, 10) to match '#0000ff', but got '#dadada'. See the artifact 'color-mismatch-31ad33d2399f94a59f546189.png' for more information.

  AssertionError: Expected color at (10, 10) to match '#0000ff', but got '#dadada'. See the artifact 'color-mismatch-31ad33d2399f94a59f546189.png' for more information.

      at ScreenCapture._expectImpl (electron/spec/lib/screen-helpers.ts:192:13)

      at Context.<anonymous> (electron/spec/api-web-contents-view-spec.ts:325:11)

not ok 1431 WebContentsView setBorderRadius capture should allow resetting corners

  Expected color at (10, 10) to match '#00b140', but got '#dadada'. See the artifact 'color-mismatch-eeda5c6639338c70c5141682.png' for more information.

  AssertionError: Expected color at (10, 10) to match '#00b140', but got '#dadada'. See the artifact 'color-mismatch-eeda5c6639338c70c5141682.png' for more information.

      at ScreenCapture._expectImpl (electron/spec/lib/screen-helpers.ts:192:13)

      at Context.<anonymous> (electron/spec/api-web-contents-view-spec.ts:338:9)

not ok 1432 WebContentsView setBorderRadius capture should render when set before attached

  Expected color at (10, 10) to match '#0000ff', but got '#dadada'. See the artifact 'color-mismatch-9b9748371c36282ee5463a1b.png' for more information.

  AssertionError: Expected color at (10, 10) to match '#0000ff', but got '#dadada'. See the artifact 'color-mismatch-9b9748371c36282ee5463a1b.png' for more information.

      at ScreenCapture._expectImpl (electron/spec/lib/screen-helpers.ts:192:13)

      at Context.<anonymous> (electron/spec/api-web-contents-view-spec.ts:354:9)

That said, I don't know why this PR would be causing those tests to fail. Maybe they're just flaky?

These tests actually passed in previous runs. Maybe restarting them once more might work?

ckerr · 2025-10-31T17:18:48Z

FTBFS on macOS

link

2025-10-28T13:29:32.7398420Z build step: cxx "./obj/electron/electron_lib/electron_api_session.o"
2025-10-28T13:29:32.7398860Z siso_rule: clang/cxx
2025-10-28T13:29:32.7399040Z stderr:
2025-10-28T13:29:32.7583190Z ##[error]../../electron/shell/browser/api/electron_api_session.cc:892:22: error: no viable overloaded '='
2025-10-28T13:29:32.7584850Z   892 |             callback = base::BindOnce(
2025-10-28T13:29:32.7585290Z       |             ~~~~~~~~ ^ ~~~~~~~~~~~~~~~
2025-10-28T13:29:32.7585700Z   893 |                 [](ElectronPermissionManager::StatusCallback callback,
2025-10-28T13:29:32.7586190Z       |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2025-10-28T13:29:32.7586600Z   894 |                    blink::mojom::PermissionStatus /*ignored_status*/) {
2025-10-28T13:29:32.7587070Z       |                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2025-10-28T13:29:32.7587480Z   895 |                   // Always deny regardless of what
2025-10-28T13:29:32.7587820Z       |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2025-10-28T13:29:32.7588310Z   896 |                   // blink::mojom::PermissionStatus is passed here
2025-10-28T13:29:32.7588690Z       |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2025-10-28T13:29:32.7589160Z   897 |                   std::move(callback).Run(content::PermissionResult(
2025-10-28T13:29:32.7589590Z       |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2025-10-28T13:29:32.7590030Z   898 |                       blink::mojom::PermissionStatus::DENIED,
2025-10-28T13:29:32.7590390Z       |                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2025-10-28T13:29:32.7590850Z   899 |                       content::PermissionStatusSource::UNSPECIFIED));
2025-10-28T13:29:32.7591260Z       |                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2025-10-28T13:29:32.7591600Z   900 |                 },
2025-10-28T13:29:32.7591800Z       |                 ~~
2025-10-28T13:29:32.7592290Z   901 |                 std::move(original_callback));
2025-10-28T13:29:32.7592640Z       |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2025-10-28T13:29:32.7593860Z ../../base/functional/callback.h:90:17: note: candidate function not viable: no known conversion from 'OnceCallback<UnboundRunType>' to 'const OnceCallback<void (content::PermissionResult)>' for 1st argument
2025-10-28T13:29:32.7594710Z    90 |   OnceCallback& operator=(const OnceCallback&) = delete;
2025-10-28T13:29:32.7595180Z       |                 ^         ~~~~~~~~~~~~~~~~~~~
2025-10-28T13:29:32.7596170Z ../../base/functional/callback.h:95:17: note: candidate function not viable: no known conversion from 'OnceCallback<UnboundRunType>' to 'OnceCallback<void (content::PermissionResult)>' for 1st argument
2025-10-28T13:29:32.7597020Z    95 |   OnceCallback& operator=(OnceCallback&&) noexcept = default;
2025-10-28T13:29:32.7597440Z       |                 ^         ~~~~~~~~~~~~~~
2025-10-28T13:29:32.7598890Z ../../base/functional/callback.h:104:17: note: candidate function not viable: no known conversion from 'CallbackType' (aka 'base::OnceCallback<void (blink::mojom::PermissionStatus)>') to 'RepeatingCallback<RunType>' (aka 'RepeatingCallback<void (content::PermissionResult)>') for 1st argument
2025-10-28T13:29:32.7600010Z   104 |   OnceCallback& operator=(RepeatingCallback<RunType> other) {
2025-10-28T13:29:32.7600620Z       |                 ^         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2025-10-28T13:29:32.7601860Z ../../base/functional/callback.h:196:27: note: candidate function not viable: no known conversion from 'CallbackType' (aka 'base::OnceCallback<void (blink::mojom::PermissionStatus)>') to 'internal::NullCallbackTag' for 1st argument
2025-10-28T13:29:32.7602790Z   196 |   constexpr OnceCallback& operator=(internal::NullCallbackTag) {
2025-10-28T13:29:32.7603190Z       |                           ^         ~~~~~~~~~~~~~~~~~~~~~~~~~
2025-10-28T13:29:32.7604780Z ../../base/functional/callback.h:204:27: note: candidate function not viable: no known conversion from 'CallbackType' (aka 'base::OnceCallback<void (blink::mojom::PermissionStatus)>') to 'internal::NullCallbackTag::WithSignature<RunType>' (aka 'internal::NullCallbackTag::WithSignature<void (content::PermissionResult)>') for 1st argument
2025-10-28T13:29:32.7605770Z   204 |   constexpr OnceCallback& operator=(
2025-10-28T13:29:32.7606130Z       |                           ^
2025-10-28T13:29:32.7606480Z   205 |       internal::NullCallbackTag::WithSignature<RunType>) {
2025-10-28T13:29:32.7606960Z       |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2025-10-28T13:29:32.7608120Z ../../base/functional/callback.h:214:27: note: candidate function not viable: no known conversion from 'CallbackType' (aka 'base::OnceCallback<void (blink::mojom::PermissionStatus)>') to 'internal::DoNothingCallbackTag' for 1st argument
2025-10-28T13:29:32.7609030Z   214 |   constexpr OnceCallback& operator=(internal::DoNothingCallbackTag)
2025-10-28T13:29:32.7609460Z       |                           ^         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2025-10-28T13:29:32.7611040Z ../../base/functional/callback.h:225:27: note: candidate function not viable: no known conversion from 'CallbackType' (aka 'base::OnceCallback<void (blink::mojom::PermissionStatus)>') to 'internal::DoNothingCallbackTag::WithSignature<RunType>' (aka 'internal::DoNothingCallbackTag::WithSignature<void (content::PermissionResult)>') for 1st argument
2025-10-28T13:29:32.7612150Z   225 |   constexpr OnceCallback& operator=(
2025-10-28T13:29:32.7612450Z       |                           ^
2025-10-28T13:29:32.7612880Z   226 |       internal::DoNothingCallbackTag::WithSignature<RunType>)
2025-10-28T13:29:32.7613310Z       |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2025-10-28T13:29:32.7614260Z ../../base/functional/callback.h:241:27: note: candidate template ignored: could not match 'internal::DoNothingCallbackTag::WithBoundArguments' against 'base::OnceCallback'
2025-10-28T13:29:32.7614880Z   241 |   constexpr OnceCallback& operator=(
2025-10-28T13:29:32.7615290Z       |                           ^
2025-10-28T13:29:32.7615530Z 1 error generated.

This PR has been around for quite awhile though. Do we want to land it?

If so, then we should update and land it 🙂 otherwise we should close it as-is

release-clerk · 2025-11-13T15:39:06Z

Release Notes Persisted

Added --disable-geolocation command-line flag for macOS apps to disable location services

nikwen · 2025-11-13T18:57:46Z

Sorry that we took so long, @nilayarya! We messed up on some PRs where we didn't respond. We're trying to do that better going forward.

…#45934) * feat(macos): add --disable-geolocation-mac command-line flag * internally deny geolocation requests if flag set e * wrap PermissionRequestHandler instead * wrap custom handler and deny regardless of response * Update docs/api/command-line-switches.md Co-authored-by: Will Anderson <will@itsananderson.com> * resolving conflicts during rebase * tests added * tests added: minor changes * move IsGeolocationDisabledViaCommandLine inside ElectronPermissionManager as a static member * test: inject fixturesPath via --boot-eval * Update shell/browser/electron_permission_manager.cc Co-authored-by: Robo <hop2deep@gmail.com> * chore: Fixup after merge * fixup after merge --------- Co-authored-by: Will Anderson <will@itsananderson.com> Co-authored-by: Robo <hop2deep@gmail.com> Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>

This causes Electron to not request GeoLocation services from macOS. We don't use it, and it looks bad to have it. The flag was added to a recent Electron version, so this change won't take effect until we update Electron. electron/electron#45934

This causes Electron to not request location services from macOS. We don't use it, and it looks bad to have it. The flag was added to a recent Electron version, so this change won't take effect until we update Electron. electron/electron#45934

electron-cation bot added the new-pr 🌱 PR opened recently label Mar 9, 2025

deepak1556 reviewed Mar 14, 2025

View reviewed changes

electron-cation bot removed the new-pr 🌱 PR opened recently label Mar 16, 2025

nilayarya marked this pull request as ready for review March 26, 2025 12:19

electron-cation bot added the new-pr 🌱 PR opened recently label Mar 26, 2025

nilayarya commented Mar 26, 2025

View reviewed changes

shell/browser/api/electron_api_session.cc Show resolved Hide resolved

nilayarya requested a review from deepak1556 March 26, 2025 15:40

jkleinsc added the semver/minor backwards-compatible functionality label Mar 31, 2025

electron-cation bot added api-review/requested 🗳 and removed new-pr 🌱 PR opened recently labels Mar 31, 2025

samuelmaddock reviewed Apr 10, 2025

View reviewed changes

itsananderson reviewed Apr 10, 2025

View reviewed changes

nilayarya changed the title ~~feat(macos): add --disable-geolocation-mac command-line flag~~ feat(macos): add --disable-geolocation command-line flag Apr 11, 2025

nilayarya requested a review from itsananderson April 11, 2025 05:29

itsananderson reviewed Apr 17, 2025

View reviewed changes

nilayarya requested review from itsananderson and samuelmaddock April 19, 2025 17:04

itsananderson approved these changes Apr 19, 2025

View reviewed changes

docs/api/command-line-switches.md Outdated Show resolved Hide resolved

electron-cation bot added api-review/approved ✅ and removed api-review/requested 🗳 labels Apr 19, 2025

nilayarya changed the title ~~feat(macos): add --disable-geolocation command-line flag~~ feat: add --disable-geolocation command-line flag for macOS Apr 19, 2025

nilayarya force-pushed the disable-geolocation-flag branch from b5ee90c to f805af0 Compare April 29, 2025 20:58

nilayarya requested a review from itsananderson April 29, 2025 21:25

itsananderson approved these changes Apr 29, 2025

View reviewed changes

deepak1556 reviewed Apr 30, 2025

View reviewed changes

shell/browser/electron_permission_manager.h Outdated Show resolved Hide resolved

spec/chromium-spec.ts Outdated Show resolved Hide resolved

nilayarya requested a review from deepak1556 April 30, 2025 12:27

nilayarya and others added 5 commits May 4, 2025 21:02

tests added

db34d6d

tests added: minor changes

db0131b

move IsGeolocationDisabledViaCommandLine inside ElectronPermissionMan…

5ad6f15

…ager as a static member

test: inject fixturesPath via --boot-eval

8e7d22d

Update shell/browser/electron_permission_manager.cc

6730da4

Co-authored-by: Robo <hop2deep@gmail.com>

nilayarya force-pushed the disable-geolocation-flag branch from e01ec30 to 6730da4 Compare May 4, 2025 15:47

jkleinsc added 2 commits October 27, 2025 15:24

Merge branch 'main' into disable-geolocation-flag

d81ad37

chore: Fixup after merge

c544e41

fixup after merge

618ff9f

jkleinsc added the no-backport label Nov 13, 2025

jkleinsc approved these changes Nov 13, 2025

View reviewed changes

jkleinsc merged commit 32ae696 into electron:main Nov 13, 2025
111 of 113 checks passed

hulthe mentioned this pull request Nov 26, 2025

Add disable-geolocation switch to Electron mullvad/mullvadvpn-app#9416

Merged


		### --disable-geolocation-mac

		Disables the Geolocation API on macOS. Has no effect on other platforms.


		Disable HTTP/2 and SPDY/3.1 protocols.

		### --disable-geolocation-mac

Conversation

nilayarya commented Mar 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of Change

Checklist

Release Notes

Uh oh!

nilayarya commented Mar 9, 2025

Uh oh!

nilayarya commented Mar 14, 2025

Uh oh!

deepak1556 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

samuelmaddock left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

itsananderson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nilayarya commented Apr 19, 2025

Uh oh!

itsananderson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nilayarya commented Apr 29, 2025

Uh oh!

deepak1556 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nilayarya commented May 4, 2025

Uh oh!

itsananderson commented May 8, 2025

Uh oh!

nilayarya commented May 8, 2025

Uh oh!

ckerr commented Oct 31, 2025

Uh oh!

Uh oh!

release-clerk bot commented Nov 13, 2025

Uh oh!

nikwen commented Nov 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

nilayarya commented Mar 9, 2025 •

edited

Loading