You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains 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
The added logic in presentInsightfulSheetsIfNeeded (lines 106-124) includes a TODO comment indicating that the logic is not ideal and should be refactored in the future. This should be reviewed to ensure it does not introduce unintended side effects or performance issues.
logger.log("Find and handle route called after didFinishLaunch after onboarding",
level:.info,
category:.coordinator)findAndHandle(route: savedRoute)}}
// MARK: - BrowserDelegate
func showLegacyHomepage(
inline:Bool,
toastContainer:UIView,
homepanelDelegate:HomePanelDelegate,
libraryPanelDelegate:LibraryPanelDelegate,
statusBarScrollDelegate:StatusBarScrollDelegate,
overlayManager:OverlayModeManager){let legacyHomepageViewController =getHomepage(
inline: inline,
Several sections of the code (e.g., lines 393-396, 432-434, 760-763) include commented-out code with Ecosia-specific changes. These should be reviewed to ensure they are necessary and do not clutter the codebase unnecessarily.
/* Ecosia: actionNeeded set to false
let isActionNeeded = RustFirefoxAccounts.shared.isActionNeeded
*/
letisActionNeeded=falseletshowWarningBadge= isActionNeeded
if isToolbarRefactorEnabled {letaction=ToolbarAction(
showMenuWarningBadge: showWarningBadge,
windowUUID: windowUUID,
actionType:ToolbarActionType.showMenuWarningBadge
)
store.dispatch(action)}else{
urlBar.warningMenuBadge(setVisible: showWarningBadge)
toolbar.warningMenuBadge(setVisible: showWarningBadge)}}func updateToolbarStateForTraitCollection(_ newCollection:UITraitCollection){letshowNavToolbar=ToolbarHelper().shouldShowNavigationToolbar(for: newCollection)letshowTopTabs=ToolbarHelper().shouldShowTopTabs(for: newCollection)switchToolbarIfNeeded()if isToolbarRefactorEnabled {if showNavToolbar {
navigationToolbarContainer.isHidden =false
navigationToolbarContainer.applyTheme(theme:currentTheme())updateTabCountUsingTabManager(self.tabManager)}else{
navigationToolbarContainer.isHidden =true}updateToolbarStateTraitCollectionIfNecessary(newCollection)}else{
urlBar.topTabsIsShowing = showTopTabs
urlBar.setShowToolbar(!showNavToolbar)
/* Ecosia: Change to circle button
toolbar.addNewTabButton.isHidden = showNavToolbar
*/
toolbar.circleButton.isHidden =falseif showNavToolbar {
toolbar.isHidden =false
toolbar.tabToolbarDelegate =self
toolbar.applyUIMode(
isPrivate: tabManager.selectedTab?.isPrivate ??false,
theme:currentTheme())
toolbar.applyTheme(theme:currentTheme())handleMiddleButtonState(currentMiddleButtonState ??.search)updateTabCountUsingTabManager(self.tabManager)}else{
toolbar.tabToolbarDelegate =nil
toolbar.isHidden =true}}appMenuBadgeUpdate()if showTopTabs, topTabsViewController ==nil{lettopTabsViewController=TopTabsViewController(tabManager: tabManager, profile: profile)
topTabsViewController.delegate =selfaddChild(topTabsViewController)
header.addArrangedViewToTop(topTabsViewController.view)self.topTabsViewController = topTabsViewController
topTabsViewController.applyTheme()}elseif showTopTabs, topTabsViewController !=nil{
topTabsViewController?.applyTheme()}else{iflet topTabsView = topTabsViewController?.view {
header.removeArrangedView(topTabsView)}
topTabsViewController?.removeFromParent()
topTabsViewController =nil}
header.setNeedsLayout()
view.layoutSubviews()iflet tab = tabManager.selectedTab,let webView = tab.webView,
!isToolbarRefactorEnabled {updateURLBarDisplayURL(tab)
navigationToolbar.updateBackStatus(webView.canGoBack)
navigationToolbar.updateForwardStatus(webView.canGoForward)}
// Ecosia: Update toolbar search/add button
toolbar.circleButton.config = isBottomSearchBar ?.newTab :.search
}func dismissVisibleMenus(){
displayedPopoverController?.dismiss(animated:true)ifself.presentedViewController as?PhotonActionSheet!=nil{self.presentedViewController?.dismiss(animated:true, completion:nil)}}@objcfunc appDidEnterBackgroundNotification(){
displayedPopoverController?.dismiss(animated:false){self.updateDisplayedPopoverProperties =nilself.displayedPopoverController =nil}ifself.presentedViewController as?PhotonActionSheet!=nil{self.presentedViewController?.dismiss(animated:true, completion:nil)}iflet tab = tabManager.selectedTab {
screenshotHelper.takeScreenshot(tab)}
// Formerly these calls were run during AppDelegate.didEnterBackground(), but we have
// individual TabManager instances for each BVC, so we perform these here instead.
tabManager.preserveTabs()logTelemetryForAppDidEnterBackground()}@objcfunc tappedTopArea(){
scrollController.showToolbars(animated:true)}@objcfunc appWillResignActiveNotification(){
// Dismiss any popovers that might be visible
displayedPopoverController?.dismiss(animated:false){self.updateDisplayedPopoverProperties =nilself.displayedPopoverController =nil}
// If we are displaying a private tab, hide any elements in the tab that we wouldn't want shown
// when the app is in the home switcher
guardlet privateTab = tabManager.selectedTab,
privateTab.isPrivate,
canShowPrivacyView
else{return}
contentStackView.alpha =0
/* Ecosia: Update background overlay for private tabs
privacyWindowHelper.showWindow(withThemedColor: currentTheme().colors.layer3)
*/
privacyWindowHelper.showWindow(withThemedColor:currentTheme().colors.ecosia.backgroundPrimary)if isToolbarRefactorEnabled {
addressToolbarContainer.alpha =0}else{
urlBar.locationContainer.alpha =0}
presentedViewController?.popoverPresentationController?.containerView?.alpha =0
presentedViewController?.view.alpha =0}varcanShowPrivacyView:Bool{
// Show privacy view if no view controller is presented
// or if the presented view is a PhotonActionSheet.
self.presentedViewController ==nil || presentedViewController is PhotonActionSheet}@objcfunc appDidBecomeActiveNotification(){
// Re-show any components that might have been hidden because they were being displayed
// as part of a private mode tab
UIView.animate(
withDuration:0.2,
delay:0,
options:UIView.AnimationOptions(),
animations:{self.contentStackView.alpha =1ifself.isToolbarRefactorEnabled {self.addressToolbarContainer.alpha =1}else{self.urlBar.locationContainer.alpha =1}self.presentedViewController?.popoverPresentationController?.containerView?.alpha =1self.presentedViewController?.view.alpha =1}, completion:{ _ inself.privacyWindowHelper.removeWindow()})iflet tab = tabManager.selectedTab, !tab.isFindInPageMode {
// Re-show toolbar which might have been hidden during scrolling (prior to app moving into the background)
scrollController.showToolbars(animated:false)}browserDidBecomeActive()}func browserDidBecomeActive(){letuuid= tabManager.windowUUID
AppEventQueue.started(.browserUpdatedForAppActivation(uuid))defer{AppEventQueue.completed(.browserUpdatedForAppActivation(uuid))}nightModeUpdates()
// Update lock icon without redrawing the whole locationView
iflet tab = tabManager.selectedTab, !isToolbarRefactorEnabled {
// It appears this was added to fix an issue with the lock icon, so we're
// calling into this for some kind of beneficial side effect. We should
// probably explore a different solution; tab content blocking does not
// change every time the app is brought forward. [FXIOS-10091]
urlBar.locationView.tabDidChangeContentBlocking(tab)}dismissModalsIfStartAtHome()}privatefunc nightModeUpdates(){ifNightModeHelper.isActivated(),
!featureFlags.isFeatureEnabled(.nightMode, checking:.buildOnly){NightModeHelper.turnOff()
themeManager.applyThemeUpdatesToWindows()}NightModeHelper.cleanNightModeDefaults()}privatefunc dismissModalsIfStartAtHome(){guard tabManager.startAtHomeCheck()else{return}letaction=FakespotAction(isOpen:false,
windowUUID: windowUUID,
actionType:FakespotActionType.setAppearanceTo)
store.dispatch(action)guard presentedViewController !=nilelse{return}dismissVC()}
// MARK: - Redux
func subscribeToRedux(){letaction=ScreenAction(windowUUID: windowUUID,
actionType:ScreenActionType.showScreen,
screen:.browserViewController)
store.dispatch(action)letbrowserAction=GeneralBrowserMiddlewareAction(
toolbarPosition: searchBarPosition,
windowUUID: windowUUID,
actionType:GeneralBrowserMiddlewareActionType.browserDidLoad)
store.dispatch(browserAction)letuuid=self.windowUUID
store.subscribe(self, transform:{
$0.select({ appState inreturnBrowserViewControllerState(appState: appState, uuid: uuid)})})}func unsubscribeFromRedux(){letaction=ScreenAction(windowUUID: windowUUID,
actionType:ScreenActionType.closeScreen,
screen:.browserViewController)
store.dispatch(action)
// Note: actual `store.unsubscribe()` is not strictly needed; Redux uses weak subscribers
}func newState(state:BrowserViewControllerState){
browserViewControllerState = state
// opens or close sidebar/bottom sheet to match the saved state
if state.fakespotState.isOpen {letproductURL= isToolbarRefactorEnabled ?
store.state.screenState(ToolbarState.self, for:.toolbar, window: windowUUID)?.addressToolbar.url :
urlBar.currentURL
guardlet productURL else{return}handleFakespotFlow(productURL: productURL)}elseif !state.fakespotState.isOpen {dismissFakespotIfNeeded()}if state.reloadWebView {updateContentInHomePanel(state.browserViewType)}setupMiddleButtonStatus(isLoading:false)iflet toast = state.toast {self.showToastType(toast: toast)}if state.showOverlay ==true{
overlayManager.openNewTab(url:nil, newTabSettings: newTabSettings)}elseif state.showOverlay ==false{
overlayManager.cancelEditing(shouldCancelLoading:false)}executeNavigationAndDisplayActions()handleMicrosurvey(state: state)}privatefunc showToastType(toast:ToastType){switch toast {case.addShortcut,.addToReadingList,.removeShortcut,.removeFromReadingList,.addBookmark:SimpleToast().showAlertWithText(
toast.title,
bottomContainer: contentContainer,
theme:currentTheme())default:letviewModel=ButtonToastViewModel(
labelText: toast.title,
buttonText: toast.buttonText)letuuid= windowUUID
lettoast=ButtonToast(viewModel: viewModel,
theme:currentTheme(),
completion:{ buttonPressed iniflet action = toast.reduxAction(for: uuid), buttonPressed {
store.dispatch(action)}})show(toast: toast)}}privatefunc handleMicrosurvey(state:BrowserViewControllerState){if !state.microsurveyState.showPrompt {guard microsurvey !=nilelse{return}removeMicrosurveyPrompt()}elseif state.microsurveyState.showSurvey {guardlet model = state.microsurveyState.model else{
logger.log("Microsurvey model should not be nil", level:.warning, category:.redux)return}
navigationHandler?.showMicrosurvey(model: model)}elseif state.microsurveyState.showPrompt {guard microsurvey ==nilelse{return}createMicrosurveyPrompt(with: state.microsurveyState)}}
// MARK: - Lifecycle
overridefunc viewDidLoad(){
super.viewDidLoad()KeyboardHelper.defaultHelper.addDelegate(self)trackTelemetry()setupNotifications()addSubviews()listenForThemeChange(view)setupAccessibleActions()
clipboardBarDisplayHandler =ClipboardBarDisplayHandler(prefs: profile.prefs, tabManager: tabManager)
clipboardBarDisplayHandler?.delegate =self
navigationToolbarContainer.toolbarDelegate =self
scrollController.header = header
scrollController.overKeyboardContainer = overKeyboardContainer
scrollController.bottomContainer = bottomContainer
updateToolbarStateForTraitCollection(traitCollection)setupConstraints()
// Setup UIDropInteraction to handle dragging and dropping
// links into the view from other apps.
letdropInteraction=UIDropInteraction(delegate:self)
view.addInteraction(dropInteraction)
/* Ecosia: Remove SearchTelemetry
searchTelemetry = SearchTelemetry(tabManager: tabManager)
*/
Logic Complexity The presentInsightfulSheetsIfNeeded function (lines 106-150) introduces a complex decision-making process for presenting sheets. This logic should be validated for correctness and maintainability.
Latest suggestions up to 51408a1
Explore these optional code suggestions:
Category
Suggestion
Impact
General
Prevent simultaneous sheet presentations
Add a safeguard to ensure that presentInsightfulSheetsIfNeeded does not execute multiple times simultaneously, which could lead to unexpected behavior.
Why: Adding a safeguard to prevent simultaneous executions of presentInsightfulSheetsIfNeeded addresses potential race conditions and ensures reliable behavior. This is a significant enhancement to the code's robustness.
High
Add fallback for theme colors
Add a fallback mechanism for currentTheme().colors.ecosia.backgroundSecondary to handle cases where the theme might not include the expected color.
Why: Adding a fallback mechanism for theme colors enhances the code's resilience by ensuring a default value is used if the expected color is unavailable. This is a practical and effective improvement.
Medium
Possible issue
Validate inline variable initialization
Ensure that the inline variable is properly defined and initialized before being used in the conditional statement to avoid potential runtime errors.
Why: The suggestion ensures that the inline variable is properly initialized before use, which prevents potential runtime errors. This is a critical improvement for code safety and correctness.
Medium
Safeguard currentURL unwrapping
Ensure that the currentURL variable is safely unwrapped before being used in the loadRequest method to prevent potential crashes.
-if let currentURL = urlBar.currentURL, let nav = tab.loadRequest(URLRequest(url: currentURL)) {+guard let currentURL = urlBar.currentURL else { return }+if let nav = tab.loadRequest(URLRequest(url: currentURL)) {
self.recordNavigationInTab(tab, navigation: nav, visitType: visitType)
}
Suggestion importance[1-10]: 7
__
Why: The suggestion ensures safe unwrapping of currentURL, which prevents potential crashes due to nil values. While the improvement is valuable, the existing code already handles the unwrapping in a concise manner.
-if let currentURL = urlBar.currentURL, let nav = tab.loadRequest(URLRequest(url: currentURL)) {+if let currentURL = urlBar.currentURL, let validURL = URL(string: currentURL.absoluteString), let nav = tab.loadRequest(URLRequest(url: validURL)) {
Suggestion importance[1-10]: 8
__
Why: Validating the URL before creating a request is crucial to prevent crashes due to malformed or nil URLs. This suggestion improves the robustness of the function and ensures safer handling of user input.
Medium
Handle empty presentation functions case
Ensure that the presentInsightfulSheetsIfNeeded function handles cases where presentationFunctions contains no valid functions to execute, to avoid potential unintended behavior or errors.
-_ = presentationFunctions.first(where: { $0() })+if presentationFunctions.first(where: { $0() }) == nil {+ // Handle case where no presentation function is executed+}
Suggestion importance[1-10]: 7
__
Why: The suggestion addresses a potential issue where no presentation function is executed, which could lead to unintended behavior. Adding a fallback ensures robustness, though the exact handling logic is not provided in the suggestion.
Medium
Handle nil navigation controller safely
Ensure that the popToBVC function properly handles cases where navigationController or topViewController is nil to avoid runtime crashes.
-guard let currentViewController = navigationController?.topViewController else { return }+guard let navigationController = navigationController, let currentViewController = navigationController.topViewController else { return }
Suggestion importance[1-10]: 6
__
Why: The suggestion ensures that the navigation controller is not nil before accessing its topViewController, which prevents potential runtime crashes. While useful, the improvement is minor as the existing code already handles nil topViewController.
Low
General
Add fallback for theme application
Add a fallback or default behavior for updateURLBarFollowingPrivateModeUI in case tabManager.selectedTab or themeManager.getCurrentTheme returns nil, to prevent potential crashes.
Why: Adding a fallback for theme application ensures that the function handles cases where the theme is nil, preventing potential crashes. This enhances the robustness of the code, though the exact fallback behavior is not specified.
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.
MOB-3155
Context
Some of Ecosia changes on
BrowserViewControllerrelated files got lost on the old project path.Approach
Review file by file and apply the relevant changes on the new project path files.
Other
Before merging
Checklist
// Ecosia:helper comments where needed