-
Notifications
You must be signed in to change notification settings - Fork 920
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
[iOS] Move web view delegate logic into a shared components #27746
Conversation
ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+TabWebDelegate.swift
Show resolved
Hide resolved
3090a2c
to
c46d993
Compare
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.
These tests were already disabled for a long time
ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+TabWebDelegate.swift
Show resolved
Hide resolved
The security team is monitoring all repositories for certain keywords. This PR includes the word(s) "policy" and so security team members have been added as reviewers to take a look. |
ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+TabWebDelegate.swift
Show resolved
Hide resolved
ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+TabWebDelegate.swift
Outdated
Show resolved
Hide resolved
ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+TabWebDelegate.swift
Show resolved
Hide resolved
...brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+TabWebPolicyDecider.swift
Show resolved
Hide resolved
4ef3c1d
to
536eb4b
Compare
ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+TabWebDelegate.swift
Show resolved
Hide resolved
536eb4b
to
4cd9a38
Compare
# Conflicts: # ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+WKNavigationDelegate.swift # Conflicts: # ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+WKNavigationDelegate.swift
4cd9a38
to
05a1e92
Compare
[puLL-Merge] - brave/brave-core@27746 DescriptionThis PR reorganizes web navigation and policy decisions in the iOS app to decouple the logic into more manageable components. The key changes include introducing protocol-based interfaces for web navigation, web policies, and user interactions. The changes also improve security by better isolating privileged calls and implementing safer protocols. ChangesChanges
sequenceDiagram
participant User
participant BVC as BrowserViewController
participant TabWebDelegate
participant TabWebNavigationDelegate
participant TabWebPolicyDecider
participant WKWebView
User->>BVC: Interacts with WebView
BVC->>TabWebDelegate: Handles User Interactions
BVC->>TabWebNavigationDelegate: Manages Navigation
TabWebNavigationDelegate->>TabWebPolicyDecider: Makes Policy Decisions
TabWebPolicyDecider->>WKWebView: Applies Navigation Policy
WKWebView-->>BVC: Navigation Result
Security Hotspots
Possible Issues
|
let touchPoint = braveWebView.lastHitPoint | ||
let touchRect = CGRect(origin: touchPoint, size: .zero) | ||
|
||
// TODO: Find a way to add fixes #3323 and #2961 here: |
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.
nit: can we prefix these with brave-ios
while we are moving them (and below in same comment)? Ex. brave-ios#3323
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.
Will address in the next PR 🙂
Released in v1.78.4 |
If uplifted please uplift #27891 |
This change moves a majority of logic that used to live in
BVC+WKNavigationDelegate.swift
into 3 separate protocols:TabWebDelegate
: Defines methods that match up withWKUIDelegate
/WKNavigationDelegate
methods, but will be expanded on to be methods that can only be handled by one implementer.TabWebNavigationDelegate
: Defines methods that match up withWKNavigationDelegate
navigational movement methods, will eventually becomeTabWebObserver
in the future and allow multiple observersTabWebPolicyDecider
: Defines the policy deciders of theWKNavigationDelegate
The biggest change is that these
Tab
prefixed methods simply pass in aTab
and no longer have access to the underlyingWKWebView
directly from the delegate method, this is to ensure we split out the WebKit-specific logic that will be only run when usingWKWebView
directly vs using Chromium web views viaCWVWebView
Resolves brave/brave-browser#44126
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
wikinpm run presubmit
wiki,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan:
Re-verify/Sanity test: