JcMinarro / Philology

An easy way to dynamically replace Strings of your Android App or provide new languages Over-the-air without needed to publish a new release on Google Play.
Apache License 2.0
511 stars 37 forks source link

TypedArray#getString does not return a translated string #28

Open ffelini opened 4 years ago

ffelini commented 4 years ago

Describe the bug When reading a custom attribute for a custom view component we are using the TypedArray returned by Context#obtainStyledAttributes. This function is invoked on a PhilologyContextWrapper provided by Philology#wrap.

The current issue with this TypedArray instance is that its getString() function does not return a translated text. That's why we need the ViewTransformer#reword where we basically obtain the resource id of the String and reapply it on the view. And that's why the android framework also fails to set a translated text from a custom attribute(we've added the support in #27 but it's still handle this manually). This is handled currently by transformer package implementations for various components.

To Reproduce Replace this reword implementation with:

    private fun TextView.reword(attributeSet: AttributeSet) {
        @StringRes val text =
            context.obtainStyledAttributes(attributeSet, intArrayOf(android.R.attr.text)).getString(0)
        @StringRes val hint =
            context.obtainStyledAttributes(attributeSet, intArrayOf(android.R.attr.hint)).getString(0)

        if (text.isNotEmpty()) setText(text)
        if (hint.isNotEmpty) setHint(hint)
    }

Run the sample and see the TextView components missing translations. Expected behavior The ideal case would be to help the framework use a correct TypedArray instance containing translated strings. This would help us get rid of ViewTransformer.reword, but this is the ideal case.

In order to ease the life of other teams would be nice to have a lint rule that will prohibit the usage of the TypedArray#getString() letting devs now that it will not return a translated text. This should be a temporary solution while we search for a real fix.

Library version v-2.1.0

ffelini commented 4 years ago

A possible hint could be the lack of Resources#obtainTypedArray implementation in PhilologyResources, maybe we should have our own implementation for it?

JcMinarro commented 4 years ago

Hi @ffelini The main problem here, and I think it is related to the #24 Issue too, is that TypeArray internally use AssetManager that is a final class on the Android Framework.

On the other hand, TypeArray.getString() works with the index of the resource that wants to be obtained, not with a valid resourceId. Maybe we could wrap the TypeArray returned, and check if the asked attribute is a string and if we have any translation for it. I can see TypeArray has a package-protected constructor, so we should create it on the android.content.res package :neutral_face:

What do you think?