Remove combinedPermissionView feature flag and old permission flow#4472
Open
tomasstrba wants to merge 9 commits intomainfrom
Open
Remove combinedPermissionView feature flag and old permission flow#4472tomasstrba wants to merge 9 commits intomainfrom
tomasstrba wants to merge 9 commits intomainfrom
Conversation
These test files tested the old storyboard-based permission UI flow (newPermissionView flag OFF) which is being permanently enabled. PopupHandlingUITests explicitly disabled the flag and tested legacy popup button context menus. PermissionsTests tested old PermissionAuthorizationViewController popover UI. Both are superseded by NewPermissionViewTests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Delete the storyboard that provided the legacy NSView-based permission UI. Clean up PermissionAuthorizationViewController to only use SwiftUI (remove IBOutlets, old updateText/IBAction methods, storyboard init). Clean up PopupBlockedPopover/ViewController similarly. The popovers no longer branch on the feature flag - they always create controllers programmatically for SwiftUI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove feature flag checks from PermissionModel, PermissionManager, and PermissionType. The new behavior is now the only behavior: - Geolocation can persist granted decisions - Popups cannot persist denied decisions - One-time decisions store .ask for permission center visibility - System permission disabled state is always checked for stored Allow - Old geolocation-disabled-on-query flow removed (dialog handles it) The featureFlagger parameter is removed from PermissionModel, PermissionManager, and PermissionType methods since it's no longer needed in the permission layer. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- AddressBarButtonsViewController: Legacy permission buttons always hidden, old button routing removed from openPermissionAuthorizationPopover, permission center button no longer guarded by flag - TabBarViewItem: Active permission icons in favicon always enabled - PrivacyDashboardPermissionHandler: Permission management fully delegated to Permission Center (old privacy dashboard permission UI code removed) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Web notifications are no longer gated behind the newPermissionView flag. The webNotifications feature flag is the only remaining gate. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove combinedPermissionView from PrivacyFeature enum - Remove newPermissionView from FeatureFlag enum and its remote mapping - Remove from bundled macos-config.json - Clean up test files: remove flag setup lines, rename test methods to drop "WhenNewPermissionViewEnabled" prefix - Remove featureFlagger from PrivacyDashboardPermissionHandler (unused) - Update comments that referenced the feature flag Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The PermissionAuthorizationPopover and PopupBlockedPopover no longer need a featureFlagger since the flag was removed. Remove the parameter from their inits and update all call sites. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The legacy per-permission buttons (camera, microphone, geolocation, popups, externalScheme, notification) in the address bar are dead code since the Permission Center replaced them. Remove: - IBOutlet declarations and height constraints - Storyboard outlet connections - Button action methods (cameraButtonAction, microphoneButtonAction, etc.) - Button setup code (position, accessibility, icons, corner radius, size) - Theme color updates for removed buttons The permissionButtons container outlet is kept to hide it on load. The storyboard views remain to avoid layout churn - they're just disconnected and always hidden. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix SwiftLint vertical_whitespace violation (double blank line)
- Update embedded config SHA after removing combinedPermissionView
- Fix PermissionModelTests for new geolocation behavior:
- Geolocation stays .active once granted (not .inactive on deactivate)
- Geolocation shows .requested query for two-step flow when system
permission is denied (not immediately .disabled)
- Navigation reset leaves geolocation in .reloading state
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tomasstrba
commented
Apr 20, 2026
| public struct Constants { | ||
| public static let embeddedDataETag = "\"ee9cd2f12a4d76f7692bc66e3233d135\"" | ||
| public static let embeddedDataSHA = "9664a46ca58ed15e793e6873201a5df56d763222eda3b4bfc8c09bbe126251b6" | ||
| public static let embeddedDataSHA = "d9867c3c702a4c5faceffa55a855f89997c331e142bd5750ac05cabeec8d6a8d" |
Contributor
Author
There was a problem hiding this comment.
This change isn't intended. I'll it back and change the configuration in its repository.
tomasstrba
commented
Apr 20, 2026
Contributor
Author
There was a problem hiding this comment.
This change isn't intended. I'll it back and change the configuration in its repository.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Task/Issue URL: https://app.asana.com/1/137249556945/project/1201037661562251/task/1214127289465210?focus=true
CC:
Description
Remove the
combinedPermissionView/newPermissionViewfeature flag and all associated old permission flow code. The flag has been enabled for a couple of months and the new SwiftUI-based permission UI is now the permanent behavior.Testing Steps
Impact and Risks
Medium - Removes old permission UI code paths. All users already see the new UI via the enabled feature flag, so functional impact should be minimal. Risk is if any edge case still relied on the old code path.
What could go wrong?
PermissionType.canPersistGrantedDecisionandcanPersistDeniedDecisionbehavior changes are now permanent (geolocation can persist granted, popups cannot persist denied)Internal references:
Definition of Done | Engineering Expectations | Tech Design Template
Note
Medium Risk
Medium risk because this removes legacy permission UI resources/tests and simplifies permission-related initialization, leaving the SwiftUI flow as the only path; any missed edge cases could surface at runtime in permission prompts/popover behavior.
Overview
Removes the legacy permissions UI and its rollout flag. The
combinedPermissionViewprivacy-config feature entry is deleted, and the oldPermissionAuthorization.storyboardplus related test targets (PermissionsTests,PopupHandlingUITests) are removed from the Xcode project.Permission flow wiring is simplified to always use the current implementation:
PermissionManagerandPermissionAuthorizationPopoverare now initialized withoutfeatureFlaggerinjection, and UI test schemes no longer skipPermissionsTests. The embedded privacy config SHA is updated accordingly.Reviewed by Cursor Bugbot for commit d5bc520. Bugbot is set up for automated code reviews on this repo. Configure here.