-
Notifications
You must be signed in to change notification settings - Fork 0
Remove FXIOS-14593 [BVC] Clean up qr code related code from BVC #22
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
base: coderabbit_combined_20260121_augment_sentry_coderabbit_1_base_remove_fxios-14593_bvc_clean_up_qr_code_related_code_from_bvc_pr206
Are you sure you want to change the base?
Remove FXIOS-14593 [BVC] Clean up qr code related code from BVC #22
Conversation
WalkthroughThis pull request comprehensively removes QR code scanning functionality from Firefox iOS across routing, UI, telemetry, and test layers. The version is bumped from 147.1 to 147.2. QR code enum cases, handlers, delegates, and metrics are deleted throughout the codebase. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (19)
💤 Files with no reviewable changes (15)
🧰 Additional context used🧬 Code graph analysis (1)firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarMiddleware.swift (2)
🔇 Additional comments (3)
✏️ Tip: You can disable this entire section by setting Comment |
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
firefox-ios/Client/Frontend/Browser/BrowserViewController/State/BrowserViewControllerState.swift (1)
559-567: Reader mode tap now routes to the long‑press sheet.Line 566 sets
displayViewto.readerModeLongPressAction, which seems intended for long‑press actions, not the tap flow. This will likely show the wrong UI when tapping Reader Mode.🔧 Proposed fix
- displayView: .readerModeLongPressAction, + displayView: .readerMode,firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarMiddleware.swift (1)
520-528: Fix telemetry mapping for reader-mode states.The current switch expression maps
.unavailableto true (via default case), which will report reader mode as enabled when it's actually unavailable. Explicit case handling is required:Fix
- let isReaderModeEnabled = switch toolbarState.addressToolbar.readerModeState { - case .available: false // will be enabled after action gets executed - default: true - } + let isReaderModeEnabled = switch toolbarState.addressToolbar.readerModeState { + case .available: + false // will be enabled after action gets executed + case .active: + true + case .unavailable: + false + }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
firefox-ios/Client/Application/QuickActions.swiftfirefox-ios/Client/Configuration/version.xcconfigfirefox-ios/Client/Coordinators/Browser/BrowserCoordinator.swiftfirefox-ios/Client/Coordinators/Router/DeeplinkInput.swiftfirefox-ios/Client/Coordinators/Router/Route.swiftfirefox-ios/Client/Coordinators/Router/RouteBuilder.swiftfirefox-ios/Client/Frontend/Browser/BrowserViewController/Actions/GeneralBrowserAction.swiftfirefox-ios/Client/Frontend/Browser/BrowserViewController/State/BrowserViewControllerState.swiftfirefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swiftfirefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarActionConfiguration.swiftfirefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarMiddleware.swiftfirefox-ios/Client/Frontend/Browser/Toolbars/ToolbarTelemetry.swiftfirefox-ios/Client/Glean/probes/metrics.yamlfirefox-ios/Client/Glean/probes/toolbar.yamlfirefox-ios/Client/Telemetry/TelemetryWrapper.swiftfirefox-ios/firefox-ios-tests/Tests/ClientTests/Coordinators/BrowserCoordinatorTests.swiftfirefox-ios/firefox-ios-tests/Tests/ClientTests/Coordinators/ShortcutRouteTests.swiftfirefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockBrowserViewController.swiftfirefox-ios/firefox-ios-tests/Tests/ClientTests/Toolbar/ToolbarMiddlewareTests.swift
💤 Files with no reviewable changes (15)
- firefox-ios/firefox-ios-tests/Tests/ClientTests/Coordinators/ShortcutRouteTests.swift
- firefox-ios/Client/Coordinators/Router/DeeplinkInput.swift
- firefox-ios/Client/Coordinators/Router/Route.swift
- firefox-ios/Client/Frontend/Browser/BrowserViewController/Actions/GeneralBrowserAction.swift
- firefox-ios/Client/Coordinators/Router/RouteBuilder.swift
- firefox-ios/firefox-ios-tests/Tests/ClientTests/Coordinators/BrowserCoordinatorTests.swift
- firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift
- firefox-ios/Client/Telemetry/TelemetryWrapper.swift
- firefox-ios/firefox-ios-tests/Tests/ClientTests/Toolbar/ToolbarMiddlewareTests.swift
- firefox-ios/Client/Glean/probes/toolbar.yaml
- firefox-ios/Client/Application/QuickActions.swift
- firefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockBrowserViewController.swift
- firefox-ios/Client/Glean/probes/metrics.yaml
- firefox-ios/Client/Coordinators/Browser/BrowserCoordinator.swift
- firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarActionConfiguration.swift
🧰 Additional context used
🧬 Code graph analysis (1)
firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarMiddleware.swift (2)
firefox-ios/Client/TabManagement/Tab.swift (1)
currentURL(1022-1024)BrowserKit/Sources/Common/Extensions/URLExtension.swift (1)
isWebPage(297-300)
🔇 Additional comments (3)
firefox-ios/Client/Configuration/version.xcconfig (1)
1-1: APP_VERSION bump looks good.No concerns with the configuration update.
firefox-ios/Client/Frontend/Browser/Toolbars/ToolbarTelemetry.swift (1)
8-12: Telemetry wrapper exposure is fine.This looks like a reasonable change for injection/testing.
firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarMiddleware.swift (1)
482-486: Reader mode URL retention on cancel-edit looks good.Keeping Reader Mode URLs prevents dropping the visible content when dismissing edit mode.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Benchmark PR from qodo-benchmark#206
Summary by CodeRabbit
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.