-
Notifications
You must be signed in to change notification settings - Fork 34
New permission authorization and new permission center #2700
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
Conversation
…oller.swift Co-authored-by: Copilot <[email protected]>
…-browsers into tom/new-permission-view
f0bec18 to
fbab23a
Compare
diegoreymendez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @tomasstrba - I found an issue while testing. See the video.
Also it seems to me that removing 1 row from the new panel reloads the page and removes all rows.
Screen.Recording.2025-12-02.at.15.12.38.mov
diegoreymendez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a few more comments.
macOS/DuckDuckGo/Permissions/View/PermissionAuthorizationSwiftUIView.swift
Show resolved
Hide resolved
macOS/DuckDuckGo/Permissions/View/PermissionAuthorizationSwiftUIView.swift
Show resolved
Hide resolved
|
@diegoreymendez, thank you for all comments, they were all very useful and I addressed them. To the main issue you mentioned above: I implemented reloading of website after removing a permission on purpose, because revoking the permission in webview didn't seem to work. Unfortunately, as a side effect, if there were any other permissions, those were dropped too. I've investigated it deeper and found out that the testing website doesn't communicate the permission correctly. After clearing the permission now, you can observe the system icon from status bar disappears, but the permission stays green on website The PR is ready for another look |
The issue is the UX is breaking the "visual contract" - the UI is saying users can remove permissions individually but they can't. Is there a task where this is discussed / documented? If not, can you open one? Once a task is up we can move forward here. |
|
@diegoreymendez, apologies if I wasn't clear enough. I've removed the tab reloading and now users can remove permissions individually. |
|
Documenting our DM conversation: removing the reload is the right call, the issue is just that the site isn't refreshing properly, but that's on the site and it looks the same in Safari and Chrome. Will be testing and hope to approve soon. |
|
Hey @tomasstrba - one more issue: when the feature is enabled, if the app is open and running, I can easily get a blank icon on the address bar. People using the app when the flag becomes enabled are likely to hit this case.
|
|
@diegoreymendez, if possible I'd address this issue in a follow-up PR. The new branch already contains some changes to presenting privacy center, which is more robust. Would that work for you? |
diegoreymendez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving.
@tomasstrba - can we ref the follow-up task here for future context?
|
@diegoreymendez, I've created the task here -> Address empty permission center button empty state issue Thank you for a great review! |


Task/Issue URL: https://app.asana.com/1/137249556945/project/1148564399326804/task/1212226055102691?focus=true
Description
This PR adds a new permission authorization flow and basic functionality of new permission center, which unblocks the notifications API work.
Unit tests, UI tests and support of popups will be added in a follow up PR.
Testing Steps
Test external schemes
"Always Allow" Flow
"Never Allow" Flow
Test microphone
"Always Allow" Flow
tccutil reset Microphone com.duckduckgo.macos.browser.debugin Terminal"Never Allow" Flow
"Deny" System Permission Flow
tccutil reset Microphone com.duckduckgo.macos.browser.debugin TerminalNote: Unfortunately, there is no callback from webview in case this happens, so we cannot trigger an info dialog if system permission is disabled for now. This applies to current behavior of the app as well. Investigation to address this will be scoped as a follow-up.
Test camera
Apply the same set of tests as microphone
To reset system permissions, execute
tccutil reset Camera com.duckduckgo.macos.browser.debugTest geolocation
Unfortunately, I haven't found an easy reliable way to to clear geolocation system permissions. Let's use a unique bundle id for each test case.
"Always Allow" Flow
DeveloperID.xcconfigfor example:MAIN_BUNDLE_IDENTIFIER[config=Debug][sdk=*] = $(MAIN_BUNDLE_IDENTIFIER_PREFIX).debughnewPermissionViewfeature flag"Always Deny" Flow
"Deny" System Permission Flow
DeveloperID.xcconfigfor example:MAIN_BUNDLE_IDENTIFIER[config=Debug][sdk=*] = $(MAIN_BUNDLE_IDENTIFIER_PREFIX).debugjnewPermissionViewfeature flagTest permission center
Test clearing permission
Test changing permission
3. Click on "Camera"
4. In authorization popup, click on "Always Deny"
5. Verify the permission was not granted ("Camera" button is red)
6. Open permission center
7. Verify the Camera permission is in the list
8. Change to "Always Allow"
9. Open https://permission.site on another tab
10. Click on "Camera"
11. Verify the permission was automatically granted
Impact and Risks
What could go wrong?
Changes are hidden behind the feature flag, but current permission functionality is still at the risk
Internal references:
Definition of Done | Engineering Expectations | Tech Design Template
Note
Introduces a new SwiftUI permission authorization flow and a Permission Center popover (gated by feature flag), adds two-step geolocation handling, and refactors permission logic to support removable/persisted decisions.
newPermissionView):PermissionAuthorizationSwiftUIViewand integrate intoPermissionAuthorizationViewController/PermissionAuthorizationPopover.PermissionCenterView,PermissionCenterViewModel, andPermissionCenterPopover; surface via new toolbar button inAddressBarButtonsViewController.SystemPermissionManagerto query/request system-level auth (geolocation).PermissionManagerto acceptFeatureFlagger, publish changes, and supportremovePermission.PermissionModelto takeFeatureFlagger, respect flag-dependent persistence, and supportremove(_:).PermissionType.canPersistGranted/DeniedDecisiondepend on feature flag; expose helpers for system-permission UI.featureFlaggerthroughAppDelegateandTabto permission components.permissionWarningBackgroundcolor.PermissionAuthorizationTypeTests; update existing permission tests and project files to include new sources.Written by Cursor Bugbot for commit 13003ad. This will update automatically on new commits. Configure here.