Bugfix FXIOS-16741 [Toolbar] - Weird behavior of the toolbar

[yoanarios]

📜 Tickets

Jira ticket
Github issue

💡 Description

Cause: PR #35292 (FXIOS-16590, "Migrate keyboardStateDidChange to ToolbarModernAction") replaced a single generic, bidirectional legacy action: keyboardStateDidChange, which any call site could dispatch with shouldShowKeyboard: true or false, with several narrower, one-directional modern actions (didCancelKeyboardRequest, accessoryViewVisibilityChanged) that only ever set it to false, and hardcoded cancelEditOnHomepage's branch to unconditionally false too. That was a reasonable simplification for the cases the refactor targeted, but it quietly removed the only remaining path that could set shouldShowKeyboard back to true outside of a genuinely fresh begin-edit (didStartEditingUrl/didPasteSearchTerm).

Resuming an edit after the keyboard was dismissed mid-session (e.g. scrolling the homepage while the Cancel button is still showing) is neither a fresh begin-edit nor one of the new actions' triggers — so once the generic restore-to-true mechanism was gone, there was no path left for that specific transition. It surfaced because LocationView.locationTextFieldDidBeginEditing already early-returns when isEditing is already true (to skip redundant setup), silently swallowing the one call that would have re-dispatched didStartEditingUrl. That guard itself is old and untouched by the refactor, it just went from "harmless, because something else could still restore the keyboard" to "the single point of failure," once the refactor deleted the other path.

Fix: locationTextFieldDidBeginEditing now always notifies the delegate, reusing the existing didStartEditingUrl dispatch (which already sets shouldShowKeyboard: true) instead of adding a new action or delegate method. It still skips the cosmetic text/attributedText reset when already editing, so nothing about the normal fresh-edit flow changes.

Add unit test

📝 Checklist

• I filled in the ticket numbers and a description of my work
• I updated the PR name to follow our PR naming guidelines
• I ensured unit tests pass and wrote tests for new code
• If working on UI, I checked and implemented accessibility (Dynamic Text and VoiceOver)
• If adding telemetry, I read the data stewardship requirements and will request a data review
• If adding or modifying strings, I read the guidelines and will request a string review from l10n
• If needed, I updated documentation and added comments to complex code

[mobiletest-ci-bot]

	Messages
📖	Project coverage: 44.37%

💪 Quality guardian

2 tests files modified. You're a champion of test coverage! 🚀

🧹 Tidy commit

Just 3 file(s) touched. Thanks for keeping it clean and review-friendly!

💬 Description craftsman

Great PR description! Reviewers salute you 🫡

✅ Code coverage

• No new files detected so the coverage gate wasn't run.
• No modified files had significant enough changes for the coverage gate to run.

ToolbarKit: Coverage: 75.95

File	Coverage
LocationView.swift	75.45%	✅

Generated by 🚫 Danger Swift against bb252b0

[ih-codes]

For posterity: I tested this out today. As discussed over DM, there was a bit of an editing state regression. With the changes to locationTextFieldDidBeginEditing, enterOverlayMode now fires the didStartEditingUrl action when tapping a top site, which causes the keyboard to open after navigating away from a Home new tab to a top site website. @yoanarios will be looking tomorrow. 🙏