From 23bec6543ddfa363cc7e40d5d2fff3a995f50f0d Mon Sep 17 00:00:00 2001 From: Brandon T Date: Fri, 8 Nov 2024 21:42:14 -0500 Subject: [PATCH] Fix translate state right after onboarding and when languages aren't downloaded --- .../BVC+ReaderMode.swift | 18 ++- .../BrowserViewController/BVC+Translate.swift | 3 +- .../UrlBar/TranslateURLBarButton.swift | 17 ++- .../BraveTranslateScriptHandler.swift | 114 ++++++++++-------- ios/nala/BUILD.gn | 1 + 5 files changed, 90 insertions(+), 63 deletions(-) diff --git a/ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+ReaderMode.swift b/ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+ReaderMode.swift index 5166079572c0..93842de7b0a7 100644 --- a/ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+ReaderMode.swift +++ b/ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+ReaderMode.swift @@ -159,14 +159,16 @@ extension BrowserViewController { if backList.count > 1 && backList.last?.url == readerModeURL { let playlistItem = tab.playlistItem + let translationState = tab.translationState webView.go(to: backList.last!) PlaylistScriptHandler.updatePlaylistTab(tab: tab, item: playlistItem) - self.updateTranslateURLBar(tab: tab, state: tab.translationState) + self.updateTranslateURLBar(tab: tab, state: translationState) } else if !forwardList.isEmpty && forwardList.first?.url == readerModeURL { let playlistItem = tab.playlistItem + let translationState = tab.translationState webView.go(to: forwardList.first!) PlaylistScriptHandler.updatePlaylistTab(tab: tab, item: playlistItem) - self.updateTranslateURLBar(tab: tab, state: tab.translationState) + self.updateTranslateURLBar(tab: tab, state: translationState) } else { // Store the readability result in the cache and load it. This will later move to the ReadabilityHelper. webView.evaluateSafeJavaScript( @@ -175,11 +177,12 @@ extension BrowserViewController { ) { (object, error) -> Void in if let readabilityResult = ReadabilityResult(object: object as AnyObject?) { let playlistItem = tab.playlistItem + let translationState = tab.translationState Task { @MainActor in try? await self.readerModeCache.put(currentURL, readabilityResult) if webView.load(PrivilegedRequest(url: readerModeURL) as URLRequest) != nil { PlaylistScriptHandler.updatePlaylistTab(tab: tab, item: playlistItem) - self.updateTranslateURLBar(tab: tab, state: tab.translationState) + self.updateTranslateURLBar(tab: tab, state: translationState) } } } @@ -203,19 +206,22 @@ extension BrowserViewController { if let originalURL = currentURL.decodeEmbeddedInternalURL(for: .readermode) { if backList.count > 1 && backList.last?.url == originalURL { let playlistItem = tab.playlistItem + let translationState = tab.translationState webView.go(to: backList.last!) PlaylistScriptHandler.updatePlaylistTab(tab: tab, item: playlistItem) - self.updateTranslateURLBar(tab: tab, state: tab.translationState) + self.updateTranslateURLBar(tab: tab, state: translationState) } else if !forwardList.isEmpty && forwardList.first?.url == originalURL { let playlistItem = tab.playlistItem + let translationState = tab.translationState webView.go(to: forwardList.first!) PlaylistScriptHandler.updatePlaylistTab(tab: tab, item: playlistItem) - self.updateTranslateURLBar(tab: tab, state: tab.translationState) + self.updateTranslateURLBar(tab: tab, state: translationState) } else { let playlistItem = tab.playlistItem + let translationState = tab.translationState if webView.load(URLRequest(url: originalURL)) != nil { PlaylistScriptHandler.updatePlaylistTab(tab: tab, item: playlistItem) - self.updateTranslateURLBar(tab: tab, state: tab.translationState) + self.updateTranslateURLBar(tab: tab, state: translationState) } } } diff --git a/ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+Translate.swift b/ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+Translate.swift index 5e65553e9110..467626aac3b3 100644 --- a/ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+Translate.swift +++ b/ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+Translate.swift @@ -15,8 +15,9 @@ extension BrowserViewController: BraveTranslateScriptHandlerDelegate { func updateTranslateURLBar(tab: Tab?, state: TranslateURLBarButton.TranslateState) { guard let tab = tab else { return } + tab.translationState = state + if tab === tabManager.selectedTab { - tab.translationState = state topToolbar.updateTranslateButtonState(state) showTranslateOnboarding(tab: tab) { [weak tab] translateEnabled in diff --git a/ios/brave-ios/Sources/Brave/Frontend/Browser/Toolbars/UrlBar/TranslateURLBarButton.swift b/ios/brave-ios/Sources/Brave/Frontend/Browser/Toolbars/UrlBar/TranslateURLBarButton.swift index 07167650f36a..5715ffbc41a0 100644 --- a/ios/brave-ios/Sources/Brave/Frontend/Browser/Toolbars/UrlBar/TranslateURLBarButton.swift +++ b/ios/brave-ios/Sources/Brave/Frontend/Browser/Toolbars/UrlBar/TranslateURLBarButton.swift @@ -88,11 +88,14 @@ class TranslateURLBarButton: UIButton { set(state) { _translateState = state switch _translateState { + case .unavailable: + self.isEnabled = false + self.isSelected = false case .available: self.isEnabled = true self.isSelected = false - case .unavailable: - self.isEnabled = false + case .pending: + self.isEnabled = true self.isSelected = false case .active: self.isEnabled = true @@ -123,8 +126,16 @@ class TranslateURLBarButton: UIButton { } enum TranslateState { - case available + // Page cannot be translated case unavailable + + // Translation is available, the page hasn't been translated yet + case available + + // Translation is in progress + case pending + + // Translation complete or partially complete case active } } diff --git a/ios/brave-ios/Sources/Brave/Frontend/UserContent/UserScripts/Scripts_Dynamic/ScriptHandlers/Sandboxed/BraveTranslateScriptHandler.swift b/ios/brave-ios/Sources/Brave/Frontend/UserContent/UserScripts/Scripts_Dynamic/ScriptHandlers/Sandboxed/BraveTranslateScriptHandler.swift index 2ab9a33dbcc6..6c32ac91c9b3 100644 --- a/ios/brave-ios/Sources/Brave/Frontend/UserContent/UserScripts/Scripts_Dynamic/ScriptHandlers/Sandboxed/BraveTranslateScriptHandler.swift +++ b/ios/brave-ios/Sources/Brave/Frontend/UserContent/UserScripts/Scripts_Dynamic/ScriptHandlers/Sandboxed/BraveTranslateScriptHandler.swift @@ -33,7 +33,8 @@ class BraveTranslateScriptHandler: NSObject, TabContentScript { private var urlObserver: NSObjectProtocol? fileprivate var currentLanguageInfo = BraveTranslateLanguageInfo() private static var elementScriptTask: Task = downloadElementScript() - private var translationTask: Future? + private var translationTask: (() async throws -> Void)? + private var canShowToast = false struct RequestMessage: Codable { let method: String @@ -54,8 +55,8 @@ class BraveTranslateScriptHandler: NSObject, TabContentScript { guard let self = self else { return } self.translationSession = session - if let task = self.translationTask { - try? await task.value + if let translationTask = self.translationTask, session != nil { + try? await translationTask() self.translationTask = nil } }, @@ -71,6 +72,7 @@ class BraveTranslateScriptHandler: NSObject, TabContentScript { if self.url != url { self.url = url self.isTranslationReady = false + self.canShowToast = false self.delegate?.updateTranslateURLBar(tab: self.tab, state: .unavailable) } } @@ -149,10 +151,11 @@ class BraveTranslateScriptHandler: NSObject, TabContentScript { ) let translationSession = self.translationSession ?? BraveTranslateSession() + let isTranslationRequest = BraveTranslateSession.isPhraseTranslationRequest(message) // The message is for HTML or CSS request if self.translationSession == nil - && BraveTranslateSession.isPhraseTranslationRequest(message) + && isTranslationRequest { throw BraveTranslateError.otherError } @@ -176,6 +179,15 @@ class BraveTranslateScriptHandler: NSObject, TabContentScript { ], nil ) + + if isTranslationRequest && canShowToast { + canShowToast = false + + Task { @MainActor in + self.delegate?.updateTranslateURLBar(tab: self.tab, state: .active) + self.delegate?.presentToast(tab: self.tab, languageInfo: currentLanguageInfo) + } + } } catch { Logger.module.error("Brave Translate Error: \(error)") replyHandler(nil, "Translation Error") @@ -231,8 +243,10 @@ class BraveTranslateScriptHandler: NSObject, TabContentScript { @MainActor @discardableResult - func executeChromiumFunction(_ functionName: String, args: [Any] = []) async -> Any? { - guard let webView = tab?.webView else { return nil } + func executeChromiumFunction(_ functionName: String, args: [Any] = []) async throws -> Any { + guard let webView = tab?.webView else { + throw BraveTranslateError.otherError + } let (result, error) = await webView.evaluateSafeJavaScript( functionName: "window.__gCrWeb.\(functionName)", @@ -243,7 +257,7 @@ class BraveTranslateScriptHandler: NSObject, TabContentScript { if let error = error { Logger.module.error("Unable to execute page function \(functionName) error: \(error)") - return nil + throw error } return result @@ -255,7 +269,7 @@ class BraveTranslateScriptHandler: NSObject, TabContentScript { @MainActor func guessLanguage() async -> Locale.Language? { - await executeChromiumFunction("languageDetection.detectLanguage") + try? await executeChromiumFunction("languageDetection.detectLanguage") // Language was identified by the Translate Script if let currentLanguage = currentLanguageInfo.currentLanguage.languageCode?.identifier, @@ -292,58 +306,46 @@ class BraveTranslateScriptHandler: NSObject, TabContentScript { } func startTranslation(canShowToast: Bool) { + // Translation already in progress + if tab?.translationState == .pending { + return + } + // This is necessary because if the session hasn't be initialized yet (since it's asynchronous in Apple's API), // then translation would not be possible. So we need to store this request for now, and when the session is ready // then we execute the translation. If the session is already available, we execute immediately. - self.translationTask = Future { promise in - Task { @MainActor [weak self] in - guard let self = self, - let currentLanguage = currentLanguageInfo.currentLanguage.languageCode?.identifier, - let pageLanguage = currentLanguageInfo.pageLanguage?.languageCode?.identifier, - currentLanguage != pageLanguage - else { - promise(.failure(BraveTranslateError.invalidLanguage)) - return - } - - do { - try Task.checkCancellation() - } catch { - promise(.failure(error)) - return - } - - // Normalize Chinese if necessary to Chinese (Simplified). zh-TW = Traditional - // We don't current use the components/language/ios/browser/language_detection_java_script_feature.mm - // Supported List of Languages is from: components/translate/core/browser/translate_language_list.cc - // So we have to do this for now. - await executeChromiumFunction( - "translate.startTranslation", - args: [ - pageLanguage == "zh" ? "zh-CN" : pageLanguage, - currentLanguage == "zh" ? "zh-CN" : currentLanguage, - ] - ) + self.translationTask = { @MainActor [weak self] in + guard let self, + let currentLanguage = currentLanguageInfo.currentLanguage.languageCode?.identifier, + let pageLanguage = currentLanguageInfo.pageLanguage?.languageCode?.identifier, + currentLanguage != pageLanguage + else { + throw BraveTranslateError.invalidLanguage + } - do { - try Task.checkCancellation() - } catch { - promise(.failure(error)) - return - } + try Task.checkCancellation() + + // Normalize Chinese if necessary to Chinese (Simplified). zh-TW = Traditional + // We don't current use the components/language/ios/browser/language_detection_java_script_feature.mm + // Supported List of Languages is from: components/translate/core/browser/translate_language_list.cc + // So we have to do this for now. + try await executeChromiumFunction( + "translate.startTranslation", + args: [ + pageLanguage == "zh" ? "zh-CN" : pageLanguage, + currentLanguage == "zh" ? "zh-CN" : currentLanguage, + ] + ) - if canShowToast { - self.delegate?.updateTranslateURLBar(tab: self.tab, state: .active) - self.delegate?.presentToast(tab: self.tab, languageInfo: currentLanguageInfo) - } + try Task.checkCancellation() - promise(.success(Void())) - } + self.canShowToast = canShowToast + self.delegate?.updateTranslateURLBar(tab: self.tab, state: .pending) } if translationSession != nil { Task { @MainActor in - try? await translationTask?.value + try? await translationTask?() translationTask = nil } } @@ -358,7 +360,7 @@ class BraveTranslateScriptHandler: NSObject, TabContentScript { return } - await executeChromiumFunction("translate.revertTranslation") + try? await executeChromiumFunction("translate.revertTranslation") self.delegate?.updateTranslateURLBar(tab: self.tab, state: .available) } } @@ -625,7 +627,7 @@ private class BraveTranslateSessionApple: BraveTranslateSession { #endif private struct BraveTranslateContainerView: View { - var onTranslationSessionUpdated: ((BraveTranslateSession) async -> Void)? + var onTranslationSessionUpdated: ((BraveTranslateSession?) async -> Void)? @ObservedObject var languageInfo: BraveTranslateLanguageInfo @@ -639,7 +641,13 @@ private struct BraveTranslateContainerView: View { .translationTask( .init(source: languageInfo.pageLanguage, target: languageInfo.currentLanguage), action: { session in - await onTranslationSessionUpdated?(BraveTranslateSessionApple(session: session)) + do { + try await session.prepareTranslation() + await onTranslationSessionUpdated?(BraveTranslateSessionApple(session: session)) + } catch { + Logger.module.error("Session Unavailable: \(error)") + await onTranslationSessionUpdated?(nil) + } } ) #else diff --git a/ios/nala/BUILD.gn b/ios/nala/BUILD.gn index f3ccdcd9b88a..a118d0077b8d 100644 --- a/ios/nala/BUILD.gn +++ b/ios/nala/BUILD.gn @@ -163,6 +163,7 @@ nala_icons = [ "leo.product.private-window.svg", "leo.product.speedreader.svg", "leo.product.sync.svg", + "leo.product.translate.svg", "leo.product.vpn.svg", "leo.qr.code.svg", "leo.radio.unchecked.svg",