Skip to content

Commit fb8581f

Browse files
Bugfix FXIOS-15303 [Translations Phase 2] Translation button doesn't appear when the primary device language isn't supported but a fallback language is (#32861)
* Fix translation button not shown when primary language lacks a model but a preferred fallback does When the translationLanguagePicker flag is enabled, shouldOfferTranslation now checks each preferred language in order and returns true on the first available model pair, instead of only checking the primary device language. The middleware builds the language list from the flag: a single device language when the flag is OFF (no behavior change), or the full ordered preferred list from PreferredTranslationLanguagesManager when ON. * Fix SwiftLint line length violation in TranslationsMiddleware * Fix SwiftLint closure body length violation in TranslationsMiddleware * Inject PreferredTranslationLanguagesManager and LocaleProvider into TranslationsMiddleware
1 parent 04acfca commit fb8581f

7 files changed

Lines changed: 132 additions & 25 deletions

File tree

firefox-ios/Client/Frontend/Translations/Service/TranslationsService.swift

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,21 +29,21 @@ final class TranslationsService: TranslationsServiceProtocol {
2929
self.logger = logger
3030
}
3131

32-
/// Determines whether translation should be offered to the user based on
33-
/// the detected page language and the device locale.
34-
func shouldOfferTranslation(for windowUUID: WindowUUID) async throws -> Bool {
35-
// Throwing here to capture this case in telemetry. It shouldn't happen in practice.
36-
guard let deviceLanguage = deviceLanguageCode() else {
37-
throw TranslationsServiceError.deviceLanguageUnavailable
38-
}
32+
/// Determines whether translation should be offered by checking any of the given
33+
/// preferred target languages against the detected page language.
34+
/// Returns `true` if at least one preferred language has an available model pair.
35+
/// NOTE: `fetchModels` inspects Remote Settings metadata and returns JSON data
36+
/// describing the pipeline, it does not fetch large model attachments.
37+
func shouldOfferTranslation(for windowUUID: WindowUUID, using preferredLanguages: [String]) async throws -> Bool {
38+
guard !preferredLanguages.isEmpty else { return false }
3939
let pageLanguage = try await detectPageLanguage(for: windowUUID)
40-
// Do not offer translations if device language is the same as detected page language.
41-
guard pageLanguage != deviceLanguage else { return false }
42-
// Only offer translation if we have a model pair (direct or via pivot).
43-
// NOTE: `fetchModels` inspects Remote Settings metadata and returns JSON data
44-
// describing the pipeline, it does not fetch large model attachments.
45-
guard await modelsFetcher.fetchModels(from: pageLanguage, to: deviceLanguage) != nil else { return false }
46-
return true
40+
for language in preferredLanguages {
41+
guard language != pageLanguage else { continue }
42+
if await modelsFetcher.fetchModels(from: pageLanguage, to: language) != nil {
43+
return true
44+
}
45+
}
46+
return false
4747
}
4848

4949
/// Initiates translation of the current page to the specified target language.

firefox-ios/Client/Frontend/Translations/Service/TranslationsServiceProtocol.swift

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@ import Common
77
/// A service responsible for coordinating in-page translations.
88
@MainActor
99
protocol TranslationsServiceProtocol {
10-
/// Determines whether translation should be offered to the user based on
11-
/// the detected page language and the device's current locale.
12-
func shouldOfferTranslation(for windowUUID: WindowUUID) async throws -> Bool
10+
/// Determines whether translation should be offered by checking any of the given
11+
/// preferred target languages against the detected page language.
12+
/// Returns `true` if at least one preferred language has an available model pair.
13+
func shouldOfferTranslation(for windowUUID: WindowUUID, using preferredLanguages: [String]) async throws -> Bool
1314
/// Performs translation and returns immediately.
1415
/// TODO(FXIOS-14213): We should implement a lifecycle for the service similar to `SummarizerServiceLifecycle`.
1516
/// For now `onLanguageIdentified` is used to notify caller when language detection is done.

firefox-ios/Client/Frontend/Translations/TranslationsMiddleware.swift

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ final class TranslationsMiddleware: FeatureFlaggable {
1414
private let windowManager: WindowManager
1515
private let translationsService: TranslationsServiceProtocol
1616
private let translationsTelemetry: TranslationsTelemetryProtocol
17+
private let manager: PreferredTranslationLanguagesManager
18+
private let localeProvider: LocaleProvider
1719

1820
/// Multiple windows can be open simultaneously, so we track IDs in a map.
1921
/// On iPhone, only a single window exists, so this will contain at most one entry.
@@ -33,13 +35,17 @@ final class TranslationsMiddleware: FeatureFlaggable {
3335
logger: Logger = DefaultLogger.shared,
3436
windowManager: WindowManager = AppContainer.shared.resolve(),
3537
translationsService: TranslationsServiceProtocol = TranslationsService(),
36-
translationsTelemetry: TranslationsTelemetryProtocol = TranslationsTelemetry()
38+
translationsTelemetry: TranslationsTelemetryProtocol = TranslationsTelemetry(),
39+
manager: PreferredTranslationLanguagesManager? = nil,
40+
localeProvider: LocaleProvider = SystemLocaleProvider()
3741
) {
3842
self.profile = profile
3943
self.logger = logger
4044
self.windowManager = windowManager
4145
self.translationsService = translationsService
4246
self.translationsTelemetry = translationsTelemetry
47+
self.manager = manager ?? PreferredTranslationLanguagesManager(prefs: profile.prefs)
48+
self.localeProvider = localeProvider
4349
}
4450

4551
lazy var translationsProvider: Middleware<AppState> = { state, action in
@@ -208,14 +214,30 @@ final class TranslationsMiddleware: FeatureFlaggable {
208214
return true
209215
}
210216

217+
/// Returns the list of target languages to check for translation eligibility.
218+
/// When the language picker flag is ON, returns the user's full preferred list.
219+
/// When OFF, returns only the primary device language (preserving legacy behavior).
220+
private func targetLanguagesForEligibilityCheck() async -> [String] {
221+
if featureFlags.isFeatureEnabled(.translationLanguagePicker, checking: .buildOnly) {
222+
let supported = await translationsService.fetchSupportedTargetLanguages()
223+
return manager.preferredLanguages(supportedTargetLanguages: supported)
224+
}
225+
return [localeProvider.current.languageCode].compactMap { $0 }
226+
}
227+
211228
/// Checks whether the current page in the active tab is eligible for translation,
212229
/// and if so, dispatches a toolbar action to update the translation state.
213230
private func checkTranslationsAreEligible(for action: ToolbarAction) {
214231
Task {
215232
guard action.translationConfiguration?.isTranslationFeatureEnabled == true else { return }
216233

217234
do {
218-
guard try await translationsService.shouldOfferTranslation(for: action.windowUUID) else { return }
235+
let preferredLanguages = await targetLanguagesForEligibilityCheck()
236+
let isEligible = try await translationsService.shouldOfferTranslation(
237+
for: action.windowUUID,
238+
using: preferredLanguages
239+
)
240+
guard isEligible else { return }
219241

220242
// Auto-translate handled the page load — skip the manual offer.
221243
if await self.tryAutoTranslate(for: action) { return }

firefox-ios/firefox-ios-tests/Tests/ClientTests/TranslationsTests/MockTranslationModelsFetcher.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,14 @@ final class MockTranslationModelsFetcher: TranslationModelsFetcherProtocol, @unc
1111
var modelsResult: Data?
1212
var modelBufferResult: Data?
1313
var supportedTargetLanguages: [String] = []
14+
var fetchModelsHandler: ((String, String) -> Data?)?
1415

1516
func fetchTranslatorWASM() -> Data? {
1617
return translatorWASMResult
1718
}
1819

1920
func fetchModels(from sourceLang: String, to targetLang: String) -> Data? {
20-
return modelsResult
21+
return fetchModelsHandler?(sourceLang, targetLang) ?? modelsResult
2122
}
2223

2324
func fetchModelBuffer(recordId: String) -> Data? {

firefox-ios/firefox-ios-tests/Tests/ClientTests/TranslationsTests/MockTranslationsService.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ final class MockTranslationsService: TranslationsServiceProtocol {
2828
}
2929

3030
// MARK: - TranslationsServiceProtocol
31-
func shouldOfferTranslation(for windowUUID: WindowUUID) async throws -> Bool {
31+
func shouldOfferTranslation(for windowUUID: WindowUUID, using preferredLanguages: [String]) async throws -> Bool {
3232
return try shouldOfferTranslationResult.get()
3333
}
3434

firefox-ios/firefox-ios-tests/Tests/ClientTests/TranslationsTests/TranslationsMiddlewareTests.swift

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,34 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility
125125
XCTAssertEqual(mockTranslationsTelemetry.pageLanguageIdentificationFailedCalledCount, 0)
126126
}
127127

128+
func test_urlDidChangeAction_withLanguagePickerEnabled_andEligiblePage_doesDispatchAction() throws {
129+
setTranslationsFeatureEnabled(enabled: true, languagePickerEnabled: true)
130+
let mockTranslationService = MockTranslationsService(
131+
shouldOfferTranslationResult: .success(true)
132+
)
133+
let subject = createSubject(translationsService: mockTranslationService)
134+
let action = ToolbarAction(
135+
url: URL(string: "https://www.example.com"),
136+
translationConfiguration: TranslationConfiguration(prefs: mockProfile.prefs),
137+
windowUUID: .XCTestDefaultUUID,
138+
actionType: ToolbarActionType.urlDidChange
139+
)
140+
141+
let expectation = XCTestExpectation(description: "expect receivedTranslationLanguage action to be fired")
142+
mockStore.dispatchCalled = { expectation.fulfill() }
143+
144+
subject.translationsProvider(mockStore.state, action)
145+
146+
wait(for: [expectation], timeout: 1.0)
147+
148+
let actionCalled = try XCTUnwrap(mockStore.dispatchedActions.first as? ToolbarAction)
149+
let actionType = try XCTUnwrap(actionCalled.actionType as? ToolbarActionType)
150+
151+
XCTAssertEqual(actionCalled.translationConfiguration?.state, .inactive)
152+
XCTAssertEqual(actionType, ToolbarActionType.receivedTranslationLanguage)
153+
XCTAssertEqual(mockStore.dispatchedActions.count, 1)
154+
}
155+
128156
func test_urlDidChangeAction_withError_doesNotDispatchActionAndLogsError() throws {
129157
setTranslationsFeatureEnabled(enabled: true)
130158
enum TestError: Error { case example }
@@ -837,14 +865,18 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility
837865
}
838866

839867
private func createSubject(
840-
translationsService: TranslationsServiceProtocol = MockTranslationsService()
868+
translationsService: TranslationsServiceProtocol = MockTranslationsService(),
869+
manager: PreferredTranslationLanguagesManager? = nil,
870+
localeProvider: LocaleProvider = MockLocaleProvider()
841871
) -> TranslationsMiddleware {
842872
return TranslationsMiddleware(
843873
profile: mockProfile,
844874
logger: mockLogger,
845875
windowManager: mockWindowManager,
846876
translationsService: translationsService,
847-
translationsTelemetry: mockTranslationsTelemetry
877+
translationsTelemetry: mockTranslationsTelemetry,
878+
manager: manager,
879+
localeProvider: localeProvider
848880
)
849881
}
850882

firefox-ios/firefox-ios-tests/Tests/ClientTests/TranslationsTests/TranslationsServiceTests.swift

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ final class TranslationsServiceTests: XCTestCase {
5757

5858
setupWebViewForTabManager()
5959

60-
let result = try await subject.shouldOfferTranslation(for: .XCTestDefaultUUID)
60+
let result = try await subject.shouldOfferTranslation(for: .XCTestDefaultUUID, using: [deviceLanguage])
6161
XCTAssertTrue(
6262
result,
6363
"Expected shouldOfferTranslation to be true when languages differ and models are available."
@@ -76,13 +76,64 @@ final class TranslationsServiceTests: XCTestCase {
7676

7777
setupWebViewForTabManager()
7878

79-
let result = try await subject.shouldOfferTranslation(for: .XCTestDefaultUUID)
79+
let result = try await subject.shouldOfferTranslation(for: .XCTestDefaultUUID, using: [deviceLanguage])
8080
XCTAssertFalse(
8181
result,
8282
"Expected shouldOfferTranslation to be false when no models are available."
8383
)
8484
}
8585

86+
func test_shouldOfferTranslation_returnsFalse_whenPreferredLanguagesIsEmpty() async throws {
87+
let subject = createSubject(
88+
detectedLanguage: "de",
89+
languageDetectorError: nil,
90+
modelsAvailable: true
91+
)
92+
93+
setupWebViewForTabManager()
94+
95+
let result = try await subject.shouldOfferTranslation(for: .XCTestDefaultUUID, using: [])
96+
XCTAssertFalse(
97+
result,
98+
"Expected shouldOfferTranslation to be false when preferred languages list is empty."
99+
)
100+
}
101+
102+
func test_shouldOfferTranslation_returnsFalse_whenAllPreferredLanguagesMatchPageLanguage() async throws {
103+
let subject = createSubject(
104+
detectedLanguage: "de",
105+
languageDetectorError: nil,
106+
modelsAvailable: true
107+
)
108+
109+
setupWebViewForTabManager()
110+
111+
let result = try await subject.shouldOfferTranslation(for: .XCTestDefaultUUID, using: ["de"])
112+
XCTAssertFalse(
113+
result,
114+
"Expected shouldOfferTranslation to be false when the only preferred language matches the page language."
115+
)
116+
}
117+
118+
func test_shouldOfferTranslation_returnsTrue_whenFallbackPreferredLanguageHasModel() async throws {
119+
let subject = createSubject(
120+
detectedLanguage: "de",
121+
languageDetectorError: nil,
122+
modelsAvailable: false
123+
)
124+
mockModelsFetcher.fetchModelsHandler = { _, targetLang in
125+
return targetLang == "en" ? Data() : nil
126+
}
127+
128+
setupWebViewForTabManager()
129+
130+
let result = try await subject.shouldOfferTranslation(for: .XCTestDefaultUUID, using: ["fr", "en"])
131+
XCTAssertTrue(
132+
result,
133+
"Expected shouldOfferTranslation to be true when a fallback preferred language has an available model."
134+
)
135+
}
136+
86137
func test_shouldOfferTranslation_propagatesLanguageDetectorError() async {
87138
enum TestError: Error, Equatable { case example }
88139

0 commit comments

Comments
 (0)