Skip to content

Commit

Permalink
Closes mozilla-mobile#7879 Provide an error message when
Browse files Browse the repository at this point in the history
non translation is found for an add-on field.
  • Loading branch information
Amejia481 committed Oct 26, 2020
1 parent c3e61e0 commit c2f74cf
Show file tree
Hide file tree
Showing 17 changed files with 67 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import mozilla.components.concept.engine.webextension.EnableSource
import mozilla.components.feature.addons.Addon
import mozilla.components.feature.addons.R
import mozilla.components.feature.addons.migration.SupportedAddonsChecker.Frequency
import mozilla.components.feature.addons.ui.translatedName
import mozilla.components.feature.addons.ui.translateName
import mozilla.components.feature.addons.update.GlobalAddonDependencyProvider
import mozilla.components.feature.addons.worker.shouldReport
import mozilla.components.support.base.ids.SharedIdsHelper
Expand Down Expand Up @@ -192,7 +192,7 @@ internal class SupportedAddonsWorker(
return NotificationCompat.Builder(context, channelId)
.setSmallIcon(mozilla.components.ui.icons.R.drawable.mozac_ic_extensions)
.setContentTitle(getNotificationTitle(plural = newSupportedAddons.size > 1))
.setContentText(getNotificationBody(newSupportedAddons))
.setContentText(getNotificationBody(newSupportedAddons, context))
.setPriority(NotificationCompat.PRIORITY_MAX)
.setContentIntent(createContentIntent())
.setAutoCancel(true)
Expand All @@ -212,19 +212,19 @@ internal class SupportedAddonsWorker(
}

@VisibleForTesting
internal fun getNotificationBody(newSupportedAddons: List<Addon>): String? {
internal fun getNotificationBody(newSupportedAddons: List<Addon>, context: Context): String? {
return when (newSupportedAddons.size) {
0 -> null
1 -> {
val addonName = newSupportedAddons.first().translatedName
val addonName = newSupportedAddons.first().translateName(context)
applicationContext.getString(
R.string.mozac_feature_addons_supported_checker_notification_content_one,
addonName,
applicationContext.appName)
}
2 -> {
val firstAddonName = newSupportedAddons.first().translatedName
val secondAddonName = newSupportedAddons[1].translatedName
val firstAddonName = newSupportedAddons.first().translateName(context)
val secondAddonName = newSupportedAddons[1].translateName(context)
applicationContext.getString(
R.string.mozac_feature_addons_supported_checker_notification_content_two,
firstAddonName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ class AddonInstallationDialogFragment : AppCompatDialogFragment() {
rootView.findViewById<TextView>(R.id.title).text =
requireContext().getString(
R.string.mozac_feature_addons_installed_dialog_title,
addon.translatedName,
addon.translateName(requireContext()),
requireContext().appName
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,13 +201,13 @@ class AddonsManagerAdapter(

holder.titleView.text =
if (addon.translatableName.isNotEmpty()) {
addon.translatedName
addon.translateName(context)
} else {
addon.id
}

if (addon.translatableSummary.isNotEmpty()) {
holder.summaryView.text = addon.translatedSummary
holder.summaryView.text = addon.translateSummary(context)
} else {
holder.summaryView.visibility = View.GONE
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,17 @@ private val dateParser = SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'Z'", Locale.ROO
/**
* A shortcut to get the localized name of an add-on.
*/
val Addon.translatedName: String get() = translatableName.translate(this)
fun Addon.translateName(context: Context): String = translatableName.translate(this, context)

/**
* A shortcut to get the localized summary of an add-on.
*/
val Addon.translatedSummary: String get() = translatableSummary.translate(this)
fun Addon.translateSummary(context: Context): String = translatableSummary.translate(this, context)

/**
* A shortcut to get the localized description of an add-on.
*/
val Addon.translatedDescription: String get() = translatableDescription.translate(this)
fun Addon.translateDescription(context: Context): String = translatableDescription.translate(this, context)

/**
* The date the add-on was created, as a JVM date object.
Expand All @@ -57,9 +57,11 @@ val Addon.updatedAtDate: Date get() = dateParser.parse(updatedAt)!!
/**
* Try to find the default language on the map otherwise defaults to [Addon.DEFAULT_LOCALE].
*/
internal fun Map<String, String>.translate(addon: Addon): String {
internal fun Map<String, String>.translate(addon: Addon, context: Context): String {
val lang = Locale.getDefault().language
return get(lang) ?: getValue(addon.defaultLocale)
return get(lang) ?: getOrElse(addon.defaultLocale) {
context.getString(R.string.mozac_feature_addons_failed_to_translate, lang, addon.defaultLocale)
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ class PermissionsDialogFragment : AppCompatDialogFragment() {
rootView.findViewById<TextView>(R.id.title).text =
requireContext().getString(
R.string.mozac_feature_addons_permissions_dialog_title,
addon.translatedName
addon.translateName(requireContext())
)
rootView.findViewById<TextView>(R.id.permissions).text = buildPermissionsText()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class UnsupportedAddonsAdapter(

holder.titleView.text =
if (addon.translatableName.isNotEmpty()) {
addon.translatedName
addon.translateName(holder.titleView.context)
} else {
addon.id
}
Expand Down
2 changes: 2 additions & 0 deletions components/feature/addons/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,8 @@
<string name="mozac_add_on_install_progress_caption">Downloading and verifying add-on&#8230;</string>
<!-- Error shown when something unexpected happened while trying to get the add-on list from the server -->
<string name="mozac_feature_addons_failed_to_query_add_ons">Failed to query Add-ons!</string>
<!-- Error shown when unable to find a translation for an add-on field. %1$s is the locale of the user and %2$s is the default language of the add-on -->
<string name="mozac_feature_addons_failed_to_translate">Translation not found, for locale %1$s neither default language %2$s</string>
<!-- Text shown after successfully installed an add-on. %1$s is the add-on name. -->
<string name="mozac_feature_addons_successfully_installed">Successfully installed %1$s</string>
<!-- Text shown after failed to install an add-on. %1$s is the add-on name. -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import mozilla.components.feature.addons.AddonManagerException
import mozilla.components.feature.addons.R
import mozilla.components.feature.addons.update.GlobalAddonDependencyProvider
import mozilla.components.feature.addons.migration.SupportedAddonsWorker.Companion.NOTIFICATION_TAG
import mozilla.components.feature.addons.ui.translatedName
import mozilla.components.feature.addons.ui.translateName
import mozilla.components.support.base.ids.SharedIdsHelper
import mozilla.components.support.ktx.android.content.appName
import mozilla.components.support.test.mock
Expand Down Expand Up @@ -157,25 +157,25 @@ class SupportedAddonsWorkerTest {
val thirdAddon = Addon("three", translatableName = mapOf(Addon.DEFAULT_LOCALE to "three"))

// One add-on
var contentString = worker.getNotificationBody(listOf(firstAddon))
var expectedString: String? = testContext.getString(R.string.mozac_feature_addons_supported_checker_notification_content_one, firstAddon.translatedName, appName)
var contentString = worker.getNotificationBody(listOf(firstAddon), testContext)
var expectedString: String? = testContext.getString(R.string.mozac_feature_addons_supported_checker_notification_content_one, firstAddon.translateName(testContext), appName)

assertEquals(expectedString, contentString)

// Two add-ons
contentString = worker.getNotificationBody(listOf(firstAddon, secondAddon))
expectedString = testContext.getString(R.string.mozac_feature_addons_supported_checker_notification_content_two, firstAddon.translatedName, secondAddon.translatedName, appName)
contentString = worker.getNotificationBody(listOf(firstAddon, secondAddon), testContext)
expectedString = testContext.getString(R.string.mozac_feature_addons_supported_checker_notification_content_two, firstAddon.translateName(testContext), secondAddon.translateName(testContext), appName)

assertEquals(expectedString, contentString)

// More than two add-ons
contentString = worker.getNotificationBody(listOf(firstAddon, secondAddon, thirdAddon))
contentString = worker.getNotificationBody(listOf(firstAddon, secondAddon, thirdAddon), testContext)
expectedString = testContext.getString(R.string.mozac_feature_addons_supported_checker_notification_content_more_than_two, appName)

assertEquals(expectedString, contentString)

// Zero add-ons :(
contentString = worker.getNotificationBody(emptyList())
contentString = worker.getNotificationBody(emptyList(), testContext)
expectedString = null

assertEquals(expectedString, contentString)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class AddonInstallationDialogFragmentTest {
doReturn(testContext).`when`(fragment).requireContext()
val dialog = fragment.onCreateDialog(null)
dialog.show()
val name = addon.translatedName
val name = addon.translateName(testContext)
val titleTextView = dialog.findViewById<TextView>(R.id.title)
val description = dialog.findViewById<TextView>(R.id.description)
val allowedInPrivateBrowsing = dialog.findViewById<AppCompatCheckBox>(R.id.allow_in_private_browsing)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,15 @@ class ExtensionsTest {

Locale.setDefault(Locale("es"))

assertEquals("nombre", addon.translatedName)
assertEquals("nombre", addon.translateName(testContext))

Locale.setDefault(Locale.GERMAN)

assertEquals("Name", addon.translatedName)
assertEquals("Name", addon.translateName(testContext))

Locale.setDefault(Locale.ENGLISH)

assertEquals("name", addon.translatedName)
assertEquals("name", addon.translateName(testContext))
}

@Test
Expand All @@ -61,15 +61,24 @@ class ExtensionsTest {

Locale.setDefault(Locale("es"))

assertEquals("Hola", map.translate(addon))
assertEquals("Hola", map.translate(addon, testContext))

Locale.setDefault(Locale.GERMAN)

assertEquals("Hallo", map.translate(addon))
assertEquals("Hallo", map.translate(addon, testContext))

Locale.setDefault(Locale.ITALIAN)

assertEquals("Hello", map.translate(addon))
assertEquals("Hello", map.translate(addon, testContext))

Locale.setDefault(Locale.CHINESE)

val locales = mapOf("es" to "Hola", "de" to "Hallo")

val lang = Locale.getDefault().language
val notFoundTranslation = testContext.getString(R.string.mozac_feature_addons_failed_to_translate, lang, addon.defaultLocale)

assertEquals(notFoundTranslation, locales.translate(addon, testContext))
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class PermissionsDialogFragmentTest {
val dialog = fragment.onCreateDialog(null)
dialog.show()

val name = addon.translatedName
val name = addon.translateName(testContext)
val titleTextView = dialog.findViewById<TextView>(R.id.title)
val permissionTextView = dialog.findViewById<TextView>(R.id.permissions)
val permissionText = fragment.buildPermissionsText()
Expand Down
6 changes: 5 additions & 1 deletion docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ permalink: /changelog/
* ⚠️ **This is a breaking change**: Renames `blackListFile` to `blocklistFile`.
* ⚠️ **This is a breaking change**: Renames `whiteListFile` to `safelistFile`.

* **feature-addons**
* 🚒 Bug fixed [issue #7879](https://github.com/mozilla-mobile/android-components/issues/7879) Crash when the default locale is not part of the translations fields of an add-on
* ⚠️ Removed `Addon.translatedName`, `Addon.translatedSummary` and `Addon.translatedDescription` and added `Addon.translateName(context: Context)`, `Addon.translateSummary(context: Context)` and `Addon.translateDescription(context: Context)`

* **concept-engine**
* ⚠️ Removed `TrackingCategory`.`SHIMMED`, for user usability reasons, we are going to mark SHIMMED categories as blocked, to follow the same pattern as Firefox desktop for more information see [#8769](https://github.com/mozilla-mobile/android-components/issues/8769)

Expand Down Expand Up @@ -2920,7 +2924,7 @@ permalink: /changelog/
* Added custom notification icon for `FetchDownloadManager`.

* **feature-app-links**
* Added safelist for schemes of URLs to open with an external app. This defaults to `mailto`, `market`, `sms` and `tel`.
* Added whitelist for schemes of URLs to open with an external app. This defaults to `mailto`, `market`, `sms` and `tel`.

* **feature-accounts**
* ⚠️ **This is a breaking change**: Public API for interacting with `FxaAccountManager` and sync changes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import mozilla.components.feature.addons.Addon
import mozilla.components.feature.addons.ui.showInformationDialog
import mozilla.components.feature.addons.ui.translatedDescription
import mozilla.components.feature.addons.ui.translatedName
import mozilla.components.feature.addons.ui.translateName
import mozilla.components.feature.addons.ui.translateDescription
import mozilla.components.feature.addons.update.DefaultAddonUpdater
import org.mozilla.samples.browser.R
import java.text.DateFormat
Expand All @@ -45,7 +45,7 @@ class AddonDetailsActivity : AppCompatActivity() {

private fun bind(addon: Addon) {

title = addon.translatedName
title = addon.translateName(this)

bindDetails(addon)

Expand Down Expand Up @@ -123,7 +123,7 @@ class AddonDetailsActivity : AppCompatActivity() {

private fun bindDetails(addon: Addon) {
val detailsView = findViewById<TextView>(R.id.details)
val detailsText = addon.translatedDescription
val detailsText = addon.translateDescription(this)

val parsedText = detailsText.replace("\n", "<br/>")
val text = HtmlCompat.fromHtml(parsedText, HtmlCompat.FROM_HTML_MODE_COMPACT)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import kotlinx.android.synthetic.main.fragment_add_on_settings.*
import mozilla.components.concept.engine.EngineSession
import mozilla.components.concept.engine.EngineView
import mozilla.components.feature.addons.Addon
import mozilla.components.feature.addons.ui.translatedName
import mozilla.components.feature.addons.ui.translateName
import org.mozilla.samples.browser.R
import org.mozilla.samples.browser.ext.components

Expand All @@ -30,7 +30,7 @@ class AddonSettingsActivity : AppCompatActivity() {
setContentView(R.layout.activity_add_on_settings)

val addon = requireNotNull(intent.getParcelableExtra<Addon>("add_on"))
title = addon.translatedName
title = addon.translateName(this)

supportFragmentManager
.beginTransaction()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import mozilla.components.feature.addons.ui.AddonInstallationDialogFragment
import mozilla.components.feature.addons.ui.AddonsManagerAdapter
import mozilla.components.feature.addons.ui.AddonsManagerAdapterDelegate
import mozilla.components.feature.addons.ui.PermissionsDialogFragment
import mozilla.components.feature.addons.ui.translatedName
import mozilla.components.feature.addons.ui.translateName
import org.mozilla.samples.browser.R
import org.mozilla.samples.browser.ext.components
import java.util.concurrent.CancellationException
Expand Down Expand Up @@ -197,7 +197,7 @@ class AddonsFragment : Fragment(), AddonsManagerAdapterDelegate {
Toast.makeText(
requireContext(), getString(
R.string.mozac_feature_addons_failed_to_install,
addon.translatedName
addon.translateName(requireContext())
),
Toast.LENGTH_SHORT
).show()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
import mozilla.components.feature.addons.Addon
import mozilla.components.feature.addons.AddonManagerException
import mozilla.components.feature.addons.ui.translatedName
import mozilla.components.feature.addons.ui.translateName
import org.mozilla.samples.browser.ext.components
import org.mozilla.samples.browser.BrowserActivity
import org.mozilla.samples.browser.R
Expand Down Expand Up @@ -61,7 +61,7 @@ class InstalledAddonDetailsActivity : AppCompatActivity() {
}

private fun bindUI(addon: Addon) {
title = addon.translatedName
title = addon.translateName(this)

bindEnableSwitch(addon)

Expand All @@ -87,14 +87,14 @@ class InstalledAddonDetailsActivity : AppCompatActivity() {
switch.setState(true)
Toast.makeText(
this,
getString(R.string.mozac_feature_addons_successfully_enabled, addon.translatedName),
getString(R.string.mozac_feature_addons_successfully_enabled, addon.translateName(this)),
Toast.LENGTH_SHORT
).show()
},
onError = {
Toast.makeText(
this,
getString(R.string.mozac_feature_addons_failed_to_enable, addon.translatedName),
getString(R.string.mozac_feature_addons_failed_to_enable, addon.translateName(this)),
Toast.LENGTH_SHORT
).show()
}
Expand All @@ -106,14 +106,14 @@ class InstalledAddonDetailsActivity : AppCompatActivity() {
switch.setState(false)
Toast.makeText(
this,
getString(R.string.mozac_feature_addons_successfully_disabled, addon.translatedName),
getString(R.string.mozac_feature_addons_successfully_disabled, addon.translateName(this)),
Toast.LENGTH_SHORT
).show()
},
onError = {
Toast.makeText(
this,
getString(R.string.mozac_feature_addons_failed_to_disable, addon.translatedName),
getString(R.string.mozac_feature_addons_failed_to_disable, addon.translateName(this)),
Toast.LENGTH_SHORT
).show()
}
Expand Down Expand Up @@ -177,15 +177,15 @@ class InstalledAddonDetailsActivity : AppCompatActivity() {
onSuccess = {
Toast.makeText(
this,
getString(R.string.mozac_feature_addons_successfully_uninstalled, addon.translatedName),
getString(R.string.mozac_feature_addons_successfully_uninstalled, addon.translateName(this)),
Toast.LENGTH_SHORT
).show()
finish()
},
onError = { _, _ ->
Toast.makeText(
this,
getString(R.string.mozac_feature_addons_failed_to_uninstall, addon.translatedName),
getString(R.string.mozac_feature_addons_failed_to_uninstall, addon.translateName(this)),
Toast.LENGTH_SHORT
).show()
}
Expand Down
Loading

0 comments on commit c2f74cf

Please sign in to comment.