Skip to content

[iOS] Separate URL text field & display view hierarchies - #38675

Open
kylehickinson wants to merge 1 commit into
ios-search-container-controllerfrom
ios-url-bar-split
Open

[iOS] Separate URL text field & display view hierarchies#38675
kylehickinson wants to merge 1 commit into
ios-search-container-controllerfrom
ios-url-bar-split

Conversation

@kylehickinson

@kylehickinson kylehickinson commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

With the upcoming changes to the toolbar design we want to avoid having the bottom bar toolbar playing double-duty and also owning the URL input field which users. This shifts ownership of the actual url bar input to SearchContainerViewController, adding a new SearchURLBarInputView it presents alongside the favourites/search child controllers. This simplifies a lot of the keyboard constraint logic since the toolbar only needs to collapse/expand based on the web view input not the url bar input

Test

Test matrix:

  • Bottom bar and top bar display mode
  • iPhone and iPad
  • iOS 18 & 26

Steps:

  • Verify the normal URL bar while browsing still behaves the same way
  • Verify tapping the URL bar brings up the typical input and renders the same way
  • Verify typing into the URL bar still brings up auto-complete, and the search options above still show (like history, bookmarks, etc)

Special verification of iPad on iOS 26+:

  • Verify that resizing the window below fullscreen properly adjusts the url bar like normal such that it doesnt appear under the window controls (traffic lights) that float in the top-left corner

@kylehickinson
kylehickinson requested a review from a team July 31, 2026 15:20
@kylehickinson kylehickinson self-assigned this Jul 31, 2026
@kylehickinson kylehickinson added CI/skip-android Do not run CI builds for Android CI/skip-windows-x64 Do not run CI builds for Windows x64 CI/skip-macos-arm64 Do not run CI builds for macOS arm64 CI/skip-origin Do not run CI builds for Origin labels Jul 31, 2026
@github-actions

Copy link
Copy Markdown
Contributor

[puLL-Merge] - brave/brave-core@38675

PR Review: Fullscreen Search Input Refactor

Description

Replaces TopToolbarView's in-place overlay-mode URL editing with a dedicated fullscreen SearchContainerViewController presented over the browser. Motivation: keep toolbar and web view static during URL editing, eliminate overlay-mode animation/constraint complexity, and prevent browser leaking behind editor with hardware keyboard connected.

Extracts URL input field into new SearchURLBarInputView, shared shadow container into LocationContainerView. Removes inOverlayMode state and topToolbar.enterOverlayMode/leaveOverlayMode throughout; replaced by presentSearchInput/dismissSearchInput and isSearchContainerVisible computed prop.

Possible Issues

  • shortcutButton visibility change. Old: selectedShortcut != nil ? inOverlayMode : true. New: selectedShortcut == nil. Old logic hid shortcut unless overlay AND shortcut set — button only visible in overlay mode. New shows whenever shortcut set, regardless of editing state. Behavior change; toolbar now shows shortcut button during normal browsing. Verify intended.

  • beginEditing double DispatchQueue.main.async. becomeFirstResponder/setLocation in first block, selectAllText in second. Ordering relies on FIFO main-queue dispatch. Fragile if intermediate work reorders. setLocation(search: true) fires didEnterText async — search results and select-all race. Confirm selectAllText runs after text set.

  • searchContainerDidTapVoiceSearch calls topToolbarDidPressVoiceSearchButton(topToolbar). Old handler called leaveOverlayMode before delegate. New path: does search container get dismissed before voice search UI presents? presentSearchInput early-returns if searchContainer exists — voice/QR result seeding via applyExternalQuery depends on container staying alive. Verify dismiss/present lifecycle for QR + voice flows.

  • updateChildInsets uses inputBar.bounds.height. Called from viewDidLayoutSubviews. First layout pass may have zero bounds → insets wrong until relayout. Check favorites/search content not clipped on initial present.

  • isBrowserContentKeyboardActive reset. Set false in keyboardWillHide. If keyboard transitions (web content → search input) without hide event, stale true could mis-collapse bottom bar. Confirm hide always fires between transitions.

  • Removed modal-dismiss keyboard constraint logic (searchEngineSettingsDismissed, clearRecentSearchAlertDismissed, TabGridHostingController cases). New code gates on presentedViewController == nil. Bottom-bar keyboard constraints during those modals may regress — the removed comment explicitly warned bar "doesn't recover once modal dismissed." Verify download prompt / external-app-alert / clear-recent-search flows in bottom-bar mode.

  • pasteAndGoButton visibility on private mode. Uses UIPasteboard.general.hasStrings. No change but now always evaluated at SearchURLBarInputView init. Fine.

  • autocompleteDelegate weakened to weak. Correct fix (was strong, retain-cycle risk with new container ownership). Confirm no place relied on strong ref keeping delegate alive.

Security Hotspots

None.

Privacy Hotspots

  • pasteAndGoButton reads UIPasteboard.general.hasStrings/hasURLs at init. iOS surfaces paste notification to user when pasteboard accessed. hasStrings/hasURLs avoid triggering banner (metadata only), so acceptable. didTapPasteAndGoButton → actual read happens in delegate on tap only. No regression.
Changes

Changes

TopToolbarView.swift

  • Removed inOverlayMode, enterOverlayMode, leaveOverlayMode, animateToOverlayState, createLocationTextFieldContainer, removeLocationTextField, setLocation, submitLocation, setAutocompleteSuggestion, updateLocationBarRightView, cancelButton, all input buttons (qrCode/voiceSearch/pasteAndGo), AutocompleteTextFieldDelegate conformance.
  • New delegate method topToolbarDidRequestSearchInput replaces enter/leave/didEnterText/didSubmitText.
  • locationContainer now LocationContainerView; second-shadow logic moved there.
  • isURLBarEnabled toggles locationView.isUserInteractionEnabled (was text field).

SearchURLBarInputView.swift (new)

  • Editable URL/search input field + paste/QR/voice/cancel buttons + search icon. Owns AutocompleteTextField. Delegate protocol SearchURLBarInputViewDelegate for button actions.

LocationContainerView.swift (new)

  • Shared rounded double-shadow container extracted from TopToolbarView.

SearchContainerViewController.swift

  • Now owns inputBar (SearchURLBarInputView). Conforms to AutocompleteTextFieldDelegate + SearchURLBarInputViewDelegate. Added speechRecognizer, delegate params; removed autocompleteSuggestionHandler. Fullscreen layout; input bar floats above keyboard (bottom bar) or top. beginEditing/applyExternalQuery/setAutocompleteSuggestion/updateSearchResults methods.

BVC+ToolbarDelegate.swift

  • presentSearchInput/dismissSearchInput/isSearchContainerVisible replace displaySearchContainer/hideSearchContainer/insertSearchContainerView. New SearchContainerViewControllerDelegate conformance forwarding submit/cancel/paste/QR/voice.

BrowserViewController.swift + BVC extensions

  • All topToolbar.inOverlayModeisSearchContainerVisible; leaveOverlayMode/didClickCanceldismissSearchInput; enterOverlayModepresentSearchInput; setLocation+didEnterTextapplyExternalQuery. Added isBrowserContentKeyboardActive. Simplified bottom-bar keyboard constraint logic in updateViewConstraints.

AutocompleteTextField.swift

  • autocompleteDelegateweak.

TabLocationView.swift

  • Removed backgroundColor set (now on LocationContainerView.contentView).
sequenceDiagram
    actor User
    participant TLV as TabLocationView
    participant TTV as TopToolbarView
    participant BVC as BrowserViewController
    participant SC as SearchContainerViewController
    participant IB as SearchURLBarInputView

    User->>TLV: tap location
    TLV->>TTV: tabLocationViewDidTapLocation
    TTV->>BVC: topToolbarDidRequestSearchInput(text,pasted,search)
    BVC->>BVC: presentSearchInput
    BVC->>SC: init + addChild (fullscreen)
    BVC->>SC: beginEditing(text,search,selectAll)
    SC->>IB: becomeFirstResponder + setLocation
    IB->>SC: autocompleteTextField didEnterText
    SC->>SC: updateSearchResults / showSearchResults

    User->>IB: submit
    IB->>SC: autocompleteTextFieldShouldReturn
    SC->>BVC: searchContainer didSubmitText
    BVC->>BVC: processAddressBar
    BVC->>BVC: finishEditingAndSubmit
    BVC->>BVC: dismissSearchInput
    BVC->>SC: resignFirstResponder + removeFromParent

    User->>IB: cancel
    IB->>SC: autocompleteTextFieldDidCancel
    SC->>BVC: searchContainerDidCancel
    BVC->>BVC: dismissSearchInput
Loading

With the upcoming changes to the toolbar design we want to avoid having the bottom bar toolbar playing double-duty and also owning the URL input field which users. This shifts ownership of the actual url bar input to SearchContainerViewController, adding a new SearchURLBarInputView it presents alongside the favourites/search child controllers. This simplifies a lot of the keyboard constraint logic since the toolbar only needs to collapse/expand based on the web view input not the url bar input
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI/skip-android Do not run CI builds for Android CI/skip-macos-arm64 Do not run CI builds for macOS arm64 CI/skip-origin Do not run CI builds for Origin CI/skip-windows-x64 Do not run CI builds for Windows x64 puLL-Merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant