Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-11-24 05:11:16 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1885336 - Part 2: Couldn't Load Languages did not show when Expected r=android-reviewers,giorga,gl

Browse Source 
This patch fixes showing the Couldn't Load Languages translation error.
Part 1 of this series ensures the translations engine re-queries the language.

This patch:
* Fixes a `var` v. `val` recommendation
* Reorganized error handling
    * Session errors have priority over browser errors
    * Ensures null errors are also sent to clear state
* Proactively requests languages again when the dialog reopens and ensures error state

Differential Revision: https://phabricator.services.mozilla.com/D205473
This commit is contained in:
ohall-m 2024-04-01 13:25:22 +00:00
parent 0ec35c2407
commit d9cc111e59
8 changed files with 201 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
mobile/android/android-components/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngine.kt
View File

@ -760,8 +760,8 @@ class GeckoEngine(
TranslationsController.RuntimeTranslation.listSupportedLanguages().then(
{
if (it != null) {
var listOfFromLanguages = mutableListOf<Language>()
var listOfToLanguages = mutableListOf<Language>()
val listOfFromLanguages = mutableListOf<Language>()
val listOfToLanguages = mutableListOf<Language>()
if (it.fromLanguages != null) {
for (each in it.fromLanguages!!) {

11
mobile/android/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/engine/middleware/TranslationsMiddleware.kt
View File

@ -245,6 +245,17 @@ class TranslationsMiddleware(
supportedLanguages = it,
),
)
// Ensures error is cleared, if a tab made this request.
if (tabId != null) {
context.store.dispatch(
TranslationsAction.TranslateSuccessAction(
tabId = tabId,
operation = TranslationOperation.FETCH_SUPPORTED_LANGUAGES,
),
)
}
logger.info("Success requesting supported languages.")
},
onError = {

7
mobile/android/android-components/components/browser/state/src/test/java/mozilla/components/browser/state/engine/middleware/TranslationsMiddlewareTest.kt
View File

@ -139,6 +139,13 @@ class TranslationsMiddlewareTest {
),
)
verify(context.store, atLeastOnce()).dispatch(
TranslationsAction.TranslateSuccessAction(
tabId = tab.id,
operation = TranslationOperation.FETCH_SUPPORTED_LANGUAGES,
),
)
waitForIdle()
}

102
mobile/android/fenix/app/src/main/java/org/mozilla/fenix/translations/TranslationsDialogBinding.kt
View File

@ -11,7 +11,10 @@ import kotlinx.coroutines.flow.mapNotNull
import mozilla.components.browser.state.selector.findTab
import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.TabSessionState
import mozilla.components.browser.state.state.TranslationsBrowserState
import mozilla.components.browser.state.state.TranslationsState
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.translate.TranslationError
import mozilla.components.concept.engine.translate.initialFromLanguage
import mozilla.components.concept.engine.translate.initialToLanguage
import mozilla.components.lib.state.helpers.AbstractBinding
@ -71,13 +74,6 @@ class TranslationsDialogBinding(
)
}
// Dispatch engine level errors
if (browserTranslationsState.engineError != null) {
translationsDialogStore.dispatch(
TranslationsDialogAction.UpdateTranslationError(browserTranslationsState.engineError),
)
}
// Session Translations State Behavior (Tab)
val sessionTranslationsState = state.sessionState.translationsState
@ -131,34 +127,86 @@ class TranslationsDialogBinding(
updateStoreIfTranslated()
}
// A session error may override a browser error
if (sessionTranslationsState.translationError != null) {
val documentLangDisplayName = sessionTranslationsState.translationEngineState
?.detectedLanguages?.documentLangTag?.let { docLangTag ->
val documentLocale = Locale.forLanguageTag(docLangTag)
val userLocale = state.browserState.locale ?: LocaleManager.getSystemDefault()
LocaleUtils.getLocalizedDisplayName(
userLocale = userLocale,
languageLocale = documentLocale,
)
}
translationsDialogStore.dispatch(
TranslationsDialogAction.UpdateTranslationError(
translationError = sessionTranslationsState.translationError,
documentLangDisplayName = documentLangDisplayName,
),
)
}
sessionTranslationsState.translationDownloadSize?.let {
translationsDialogStore.dispatch(
TranslationsDialogAction.UpdateDownloadTranslationDownloadSize(it),
)
}
// Error handling requires both knowledge of browser and session state
updateTranslationError(
sessionTranslationsState = sessionTranslationsState,
browserTranslationsState = browserTranslationsState,
browserState = state.browserState,
)
}
}
/**
* Helper function for error handling, which requires considering both
* [TranslationsState] (session) and [TranslationsBrowserState] (browser or global) states.
*
* Will dispatch [TranslationsDialogAction] to update store when appropriate.
*
* Certain errors take priority over others, depending on how important they are.
* Error priority:
* 1. An error in [TranslationsBrowserState] of [EngineNotSupportedError] should be the
* highest priority to process.
* 2. An error in [TranslationsState] of [LanguageNotSupportedError] is the second highest
* priority and requires additional information.
* 3. Displayable browser errors only should be the dialog error when the session has a
* non-displayable error. (Usually expect this case where an error recovery action occurred on
* a different tab and failed.)
* 4. Displayable session errors and null errors are the default dialog error.
*
* @param sessionTranslationsState The session state to consider when dispatching errors.
* @param browserTranslationsState The browser state to consider when dispatching errors.
* @param browserState The browser state to consider when fetching information for errors.
*/
private fun updateTranslationError(
sessionTranslationsState: TranslationsState,
browserTranslationsState: TranslationsBrowserState,
browserState: BrowserState,
) {
if (browserTranslationsState.engineError is TranslationError.EngineNotSupportedError) {
// [EngineNotSupportedError] is a browser error that overrides all other errors.
translationsDialogStore.dispatch(
TranslationsDialogAction.UpdateTranslationError(browserTranslationsState.engineError),
)
} else if (sessionTranslationsState.translationError is TranslationError.LanguageNotSupportedError) {
// [LanguageNotSupportedError] is a special case where we need additional information.
val documentLangDisplayName = sessionTranslationsState.translationEngineState
?.detectedLanguages?.documentLangTag?.let { docLangTag ->
val documentLocale = Locale.forLanguageTag(docLangTag)
val userLocale = browserState.locale ?: LocaleManager.getSystemDefault()
LocaleUtils.getLocalizedDisplayName(
userLocale = userLocale,
languageLocale = documentLocale,
)
}
translationsDialogStore.dispatch(
TranslationsDialogAction.UpdateTranslationError(
translationError = sessionTranslationsState.translationError,
documentLangDisplayName = documentLangDisplayName,
),
)
} else if (browserTranslationsState.engineError?.displayError == true &&
sessionTranslationsState.translationError?.displayError != true
) {
// Browser errors should only be displayed with the session error is not displayable nor null.
translationsDialogStore.dispatch(
TranslationsDialogAction.UpdateTranslationError(browserTranslationsState.engineError),
)
} else if (sessionTranslationsState.translationError?.displayError != false) {
// Displayable session errors and null session errors should be passed to the dialog under most cases.
translationsDialogStore.dispatch(
TranslationsDialogAction.UpdateTranslationError(sessionTranslationsState.translationError),
)
}
}
private fun updateStoreIfIsTranslateProcessing() {
translationsDialogStore.dispatch(
TranslationsDialogAction.UpdateTranslationInProgress(

15
mobile/android/fenix/app/src/main/java/org/mozilla/fenix/translations/TranslationsDialogMiddleware.kt
View File

@ -26,6 +26,21 @@ class TranslationsDialogMiddleware(
action: TranslationsDialogAction,
) {
when (action) {
is TranslationsDialogAction.InitTranslationsDialog -> {
// If the languages are missing, we should attempt to fetch the supported languages.
// This will also ensure the missing languages dialog error state, if fetching fails.
if (browserStore.state.translationEngine.supportedLanguages?.fromLanguages == null ||
browserStore.state.translationEngine.supportedLanguages?.toLanguages == null
) {
browserStore.dispatch(
TranslationsAction.OperationRequestedAction(
tabId = sessionId,
operation = TranslationOperation.FETCH_SUPPORTED_LANGUAGES,
),
)
}
}
is TranslationsDialogAction.FetchDownloadFileSizeAction -> {
browserStore.dispatch(
TranslationsAction.FetchTranslationDownloadSizeAction(

11
mobile/android/fenix/app/src/main/java/org/mozilla/fenix/translations/TranslationsDialogStore.kt
View File

@ -276,16 +276,6 @@ internal object TranslationsDialogReducer {
)
}
is TranslationsDialogAction.InitTranslationsDialog -> {
state.copy(
positiveButtonType = if (state.initialTo == null || state.initialFrom == null) {
PositiveButtonType.Disabled
} else {
state.positiveButtonType
},
)
}
is TranslationsDialogAction.UpdateTranslationError -> {
state.copy(
error = action.translationError,
@ -323,6 +313,7 @@ internal object TranslationsDialogReducer {
)
}
TranslationsDialogAction.InitTranslationsDialog,
is TranslationsDialogAction.TranslateAction,
is TranslationsDialogAction.UpdatePageSettingsValue,
TranslationsDialogAction.TranslateAction,

90
mobile/android/fenix/app/src/test/java/org/mozilla/fenix/translations/TranslationsDialogBindingTest.kt
View File

@ -24,6 +24,7 @@ import mozilla.components.support.test.rule.runTestOnMain
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mockito.never
import org.mockito.Mockito.spy
import org.mockito.Mockito.verify
import org.mozilla.fenix.R
@ -305,6 +306,95 @@ class TranslationsDialogBindingTest {
)
}
@Test
fun `WHEN a non-displayable error is sent to the browserStore THEN the translation dialog store is not updated`() =
runTestOnMain {
translationsDialogStore =
spy(TranslationsDialogStore(TranslationsDialogState()))
browserStore = BrowserStore(
BrowserState(
tabs = listOf(tab),
selectedTabId = tabId,
),
)
val binding = TranslationsDialogBinding(
browserStore = browserStore,
translationsDialogStore = translationsDialogStore,
sessionId = tabId,
getTranslatedPageTitle = { localizedFrom, localizedTo ->
testContext.getString(
R.string.translations_bottom_sheet_title_translation_completed,
localizedFrom,
localizedTo,
)
},
)
binding.start()
val fetchError = TranslationError.UnknownEngineSupportError(null)
browserStore.dispatch(
TranslationsAction.EngineExceptionAction(
error = fetchError,
),
).joinBlocking()
verify(translationsDialogStore, never()).dispatch(
TranslationsDialogAction.UpdateTranslationError(fetchError),
)
}
@Test
fun `WHEN a browser and session error is sent to the browserStore THEN the session error takes priority and the translation dialog store is updated`() =
runTestOnMain {
translationsDialogStore =
spy(TranslationsDialogStore(TranslationsDialogState()))
browserStore = BrowserStore(
BrowserState(
tabs = listOf(tab),
selectedTabId = tabId,
),
)
val binding = TranslationsDialogBinding(
browserStore = browserStore,
translationsDialogStore = translationsDialogStore,
sessionId = tabId,
getTranslatedPageTitle = { localizedFrom, localizedTo ->
testContext.getString(
R.string.translations_bottom_sheet_title_translation_completed,
localizedFrom,
localizedTo,
)
},
)
binding.start()
val sessionError = TranslationError.CouldNotLoadLanguagesError(null)
browserStore.dispatch(
TranslationsAction.TranslateExceptionAction(
tabId = tab.id,
operation = TranslationOperation.FETCH_SUPPORTED_LANGUAGES,
translationError = sessionError,
),
).joinBlocking()
verify(translationsDialogStore).dispatch(
TranslationsDialogAction.UpdateTranslationError(sessionError),
)
val engineError = TranslationError.UnknownError(IllegalStateException())
browserStore.dispatch(
TranslationsAction.EngineExceptionAction(
error = engineError,
),
).joinBlocking()
verify(translationsDialogStore, never()).dispatch(
TranslationsDialogAction.UpdateTranslationError(engineError),
)
}
@Test
fun `WHEN set translation download size action sent to the browserStore THEN update translation dialog store based on operation`() =
runTestOnMain {

27
mobile/android/fenix/app/src/test/java/org/mozilla/fenix/translations/TranslationsDialogReducerTest.kt
View File

@ -129,33 +129,6 @@ class TranslationsDialogReducerTest {
assertEquals(PositiveButtonType.InProgress, updatedState.positiveButtonType)
}
@Test
fun `WHEN the reducer is called for InitTranslationsDialog THEN a new state for PositiveButtonType is returned`() {
val translationsDialogState = TranslationsDialogState()
val updatedState = TranslationsDialogReducer.reduce(
translationsDialogState,
TranslationsDialogAction.InitTranslationsDialog,
)
assertEquals(PositiveButtonType.Disabled, updatedState.positiveButtonType)
val spanishLanguage = Language("es", "Spanish")
val englishLanguage = Language("en", "English")
val translationsDialogStateTwo = TranslationsDialogState(
initialFrom = spanishLanguage,
initialTo = englishLanguage,
positiveButtonType = PositiveButtonType.Enabled,
)
val updatedStateTwo = TranslationsDialogReducer.reduce(
translationsDialogStateTwo,
TranslationsDialogAction.InitTranslationsDialog,
)
assertEquals(PositiveButtonType.Enabled, updatedStateTwo.positiveButtonType)
}
@Test
fun `WHEN the reducer is called for UpdateTranslationError THEN a new state with translation error is returned`() {
val translationsDialogState = TranslationsDialogState()