Website Permissions #165
Website Permissions #165
Conversation
There was a problem hiding this comment.
@mallexxx I looked at code from the high perspective and it looks great! 🥇 This is not a trivial problem when we consider edge cases and private API usage, but you did a really great job! The architecture looks good to me.
I’ll do a testing and second pass focused on details tomorrow.
| @@ -41,4 +42,13 @@ extension NSApplication { | |||
| return event.keyCode == 36 || event.keyCode == 76 | |||
| } | |||
|
|
|||
| func isActivePublisher() -> AnyPublisher<Bool, Never> { | |||
There was a problem hiding this comment.
LGMT! 👍 I only found few minor issues and have couple of questions below but other than that it is awesome!!! 🥇 🎖️ 🏅
The architecture and technical solution is really great! Good job Alex! 👏 👏 👏
- Is this behavior intentional when using only microphone permissions?
- Is the icon for geolocation right? Should’t it be filled?
DuckDuckGo.xcodeproj/project.pbxproj
Outdated
| @@ -1577,6 +1656,9 @@ | |||
| 4B723DEA26B0002B00E14D75 /* Data Import */, | |||
| 4B723DF826B0002B00E14D75 /* Data Export */, | |||
| 4B65143C26392483005B46EB /* Email */, | |||
| B64C84DB2692D6E80048FEBE /* Permissions */, | |||
There was a problem hiding this comment.
Just a tiny detail, could you preserve alphabetical order?
There was a problem hiding this comment.
Never noticed it was alphabetical :)
| } | ||
| } | ||
|
|
||
| func revokePermissions(_ permissions: [PermissionType], completionHandler: (() -> Void)? = nil) { |
| } | ||
|
|
||
| @objc(_webView:mediaCaptureStateDidChange:) | ||
| func webView(_ webView: WKWebView, mediaCaptureStateDidChange state: _WKMediaCaptureStateDeprecated) { |
There was a problem hiding this comment.
I was able to reproduce this multiple times. 1. I visited https://permission.site/ 2. allowed geolocation, mic, camera and closed the tab.
* Bring in using references to local build * Pull in privacy dashboard as submodule * Tracker info for the privacy dashboard * ServerTrustViewModel for the privacy dashboard * Tracker info and server trust encoded to JSON for JS layer. Enabling web inspector for the privacy dashboard * Update tracker data * Fix after rebase * Fix to add back in script handlers * Update submodules * Update to get working on latest macOS and pull in latest dashboard build * Remove macOS version checks and update privacy dashboard * Fix adding handler not content world * Update privacy dashboard to main branch Co-authored-by: Tomas Strba <tom@duckduckgo.com>
No, it's a bug, good catch! 👍
This behaviour is correct because it's a one-shot |
…mera&Mic transition to Revoked state
mallexxx
force-pushed
the
alex/permissions
branch
from
e3c8cff to
9e4e9f1
Compare
# By Alexey Martemyanov (6) and others # Via GitHub * develop: Fix geolocation in release build, minor fixes (#198) Basic Auth (#200) Fix PDF saving (#194) Support printing for macOS<11.0; Fix printing to PDF (#196) Version 0.14.1 User agent updated Update BrowserServicesKit to 7.1.2 (#192) Friday Party (#197) Version 0.14.0 Quick privacy icon animation (#188) Website Permissions (#165) Go back to close Child Tab (#187) # Conflicts: # DuckDuckGo.xcodeproj/project.pbxproj
Task/Issue URL: https://app.asana.com/0/1199237043630360/1200191553434234/f
Tech Design URL: https://app.asana.com/0/1199237043630360/1200503074536077/f
CC: @tomasstrba
Description:
This PR brings Geolocation, Camera and Microphone WebSite Permissions
Steps to test this PR:
Use
sudo /usr/libexec/PlistBuddy -c 'Delete com.duckduckgo.macos.browser.debug' /var/db/locationd/clients.plist ; sudo killall -HUP locationdto reset System Geolocation permission,sudo tccutil reset Camera com.duckduckgo.macos.browser.debugto reset Camera/Microphone permissionsInternal references:
Software Engineering Expectations
Technical Design Template
When ready for review, remember to post the PR in MM