AppCompatTextHelper.onSetTextAppearance does not set textColorHint or textColorLink [154702995]

Fixed

Bug

Status Update

No update yet.

Description

at...@monzo.com

created issue #1

Apr 22, 2020 03:58PM

Artifact used: androidx.appcompat:appcompat
Version: 1.2.0-beta01
Theme: Theme.MaterialComponents.Light.NoActionBar
Devices/Android versions: API 22 emulator

Issue

Setting the text appearance programmatically with TextViewCompat.setTextAppearance(TextView, Int) on an AppCompatEditText does not take into account theme attrs being used in ColorStateList as android:textColorHint (or android:textColorLink).

So on Lollipop, such a hint color is rendered as red (see screenshot).

Cause

Internally, it delegates to AppCompatTextHelper.onSetTextAppearance, which doesn't handle these two attributes - it only considers android:textColor:

if (Build.VERSION.SDK_INT < 23 && a.hasValue(R.styleable.TextAppearance_android_textColor)) {
    // If we're running on < API 23, the text color may contain theme references
    // so let's re-set using our own inflater
    final ColorStateList textColor = a.getColorStateList(R.styleable.TextAppearance_android_textColor);
    if (textColor != null) {
        mView.setTextColor(textColor);
    }
}

However, it does consider them in loadFromAttributes() so it seems like it's an oversight.

Feature request/bug fix

Update AppCompatTextHelper to handle theme attributes in CSLs for android:textColorHint and android:textColorLink in onSetTextAppearance() as it's done in loadFromAttributes().

Workaround

For folks in the meantime - call this too, after calling TextViewCompat.setTextAppearance().

/**
 * Reapply text color and textColorHint to support CSLs on Lollipop
 */
private fun TextView.resetTextColorsFromTextAppearance(@StyleRes textAppearance: Int) {
    if (Build.VERSION.SDK_INT >= 23) {
        // this fix is only necessary on API 21/22 because we need to load the ColorStateLists ourselves
        return
    }

    val attributes = intArrayOf(android.R.attr.textColor, android.R.attr.textColorHint)
    val typedArray = context.obtainStyledAttributes(textAppearance, attributes)
    val textColorRes = typedArray.getResourceId(attributes.indexOf(android.R.attr.textColor), -1)
    val textColorHintRes = typedArray.getResourceId(attributes.indexOf(android.R.attr.textColorHint), -1)
    typedArray.recycle()

    if (textColorRes != -1) {
        setTextColor(AppCompatResources.getColorStateList(context, textColorRes))
    }

    if (textColorHintRes != -1) {
        setHintTextColor(AppCompatResources.getColorStateList(context, textColorHintRes))
    }
}

79970016-33584e80-848a-11ea-81bf-94879c26575e.png

53 KB

View

Download

Comments

ki...@google.com <ki...@google.com> #2Apr 27, 2020 01:18PM

Assigned to ki...@google.com.

1.4.0-alpha03

still missing tint compound drawable ...

ki...@google.com <ki...@google.com> #3Apr 27, 2020 01:22PM

Marking as low-hanging bug. This seems like it should be straightfoward fix.

he...@ataulm.com <he...@ataulm.com> #4Apr 27, 2020 01:25PM

Project: platform/frameworks/support
Branch: androidx-main

commit 5184652d714e2f13390a9ff6884d8e4bd41d0f8e
Author: Alan Viverette <alanv@google.com>
Date: Tue Feb 08 16:13:05 2022

Implement TintableCompoundDrawablesView in TextView-derived widgets

Adds AppCompatMultiAutoCompleteViewTest, which is a direct copy of
AppCompatAutoCompleteViewTest. Everything else is copied from
AppCompatTextView and refactored to match the class names. There's no
new logic in this CL.

Relnote: Backports compound drawable tinting to TextView-derived widgets
Fixes: 165822337
Test: AppCompatCheckBoxTest, etc.
Change-Id: Idf98ced324a724a5cb5b9d5a2f397a3b82fad900

M appcompat/appcompat/src/main/java/androidx/appcompat/widget/AppCompatMultiAutoCompleteTextView.java
M appcompat/appcompat/src/main/java/androidx/appcompat/widget/AppCompatCheckBox.java
M appcompat/appcompat/src/androidTest/java/androidx/appcompat/widget/AppCompatAutoCompleteTextViewTest.java
M appcompat/appcompat/src/androidTest/res/layout/appcompat_togglebutton_activity.xml
M appcompat/appcompat/api/restricted_current.txt
M appcompat/appcompat/src/androidTest/AndroidManifest.xml
M appcompat/appcompat/src/androidTest/java/androidx/appcompat/widget/AppCompatCheckBoxTest.java
M appcompat/appcompat/src/androidTest/res/layout/appcompat_autocompletetextview_activity.xml
M appcompat/appcompat/src/main/java/androidx/appcompat/widget/AppCompatRadioButton.java
M appcompat/appcompat/src/main/java/androidx/appcompat/widget/AppCompatEditText.java
M appcompat/appcompat/src/main/java/androidx/appcompat/widget/AppCompatAutoCompleteTextView.java
A appcompat/appcompat/src/androidTest/res/layout/appcompat_multiautocompletetextview_activity.xml
M appcompat/appcompat/src/androidTest/java/androidx/appcompat/widget/AppCompatEditTextTest.java
M appcompat/appcompat/api/current.txt
M appcompat/appcompat/src/main/java/androidx/appcompat/widget/AppCompatToggleButton.java
A appcompat/appcompat/src/androidTest/java/androidx/appcompat/widget/AppCompatMultiAutoCompleteTextViewTest.java
M appcompat/appcompat/api/public_plus_experimental_current.txt
M appcompat/appcompat/src/androidTest/java/androidx/appcompat/widget/AppCompatToggleButtonTest.java
M appcompat/appcompat/src/main/java/androidx/appcompat/widget/AppCompatCheckedTextView.java
A appcompat/appcompat/src/androidTest/java/androidx/appcompat/widget/AppCompatMultiAutoCompleteTextViewActivity.java
M appcompat/appcompat/src/androidTest/res/layout/appcompat_checkedtextview_activity.xml
M appcompat/appcompat/src/androidTest/java/androidx/appcompat/widget/AppCompatCheckedTextViewTest.java
M appcompat/appcompat/src/androidTest/java/androidx/appcompat/widget/AppCompatRadioButtonTest.java
M appcompat/appcompat/src/androidTest/res/layout/appcompat_checkbox_activity.xml
M appcompat/appcompat/src/androidTest/res/layout/appcompat_edittext_activity.xml
M appcompat/appcompat/src/androidTest/res/layout/appcompat_radiobutton_activity.xml

https://android-review.googlesource.com/1978467

ki...@google.com <ki...@google.com> #5Apr 27, 2020 01:31PM

This is one of those cases where I am so glad that we have good tests. It's surprising how much can go wrong in a straightforward* copy/paste set of changes.

Anyway, this won't be in the next release -- just missed build cut deadline -- but it'll be out in a 1.5.0 alpha some time in the next ~month.

*actually not straightforward

he...@ataulm.com <he...@ataulm.com> #6Apr 27, 2020 01:36PM

Perfect thanks!

For us, there's no urgency as we're wrapping usages of `TextViewCompat.setTextAppearance` with our own function and applying this workaround on Lollipop, but would be great to ditch it at some point because no doubt this sort of thing is the source of subtle bugs/divergences in intended behaviour.

ki...@google.com <ki...@google.com> May 4, 2020 05:03PM

Accepted by ki...@google.com.

ki...@google.com <ki...@google.com> #7May 5, 2020 05:08PM

https://android-review.googlesource.com/c/platform/frameworks/support/+/1302103 under review

ap...@google.com <ap...@google.com> #8May 5, 2020 06:50PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 7869860590c185a90287c9b94e1de151b0c75504
Author: Kirill Grouchnikov <kirillg@google.com>
Date: Tue May 05 12:55:01 2020

Consistent resolution of text hint and link colors

Same as for changing set appearance and resolving the main
text color - do the same for text link and text hint colors.

Test: ./gradlew appcompat:appcompat:connectedCheck --info --daemon -Pandroid.testInstrumentationRunnerArguments.class=androidx.appcompat.widget.AppCompatEditTextTest
Test: ./gradlew appcompat:appcompat:connectedCheck --info --daemon -Pandroid.testInstrumentationRunnerArguments.class=androidx.appcompat.widget.AppCompatTextViewTest
Bug: 154702995
Change-Id: Iddb7f1e7f17b9bceb14c20ca0b3468b10092b8f9

M appcompat/appcompat/src/androidTest/AndroidManifest.xml
M appcompat/appcompat/src/androidTest/java/androidx/appcompat/widget/AppCompatEditTextTest.java
M appcompat/appcompat/src/androidTest/java/androidx/appcompat/widget/AppCompatTextViewTest.java
A appcompat/appcompat/src/androidTest/res/color/color_state_list_hint.xml
A appcompat/appcompat/src/androidTest/res/color/color_state_list_refs12.xml
A appcompat/appcompat/src/androidTest/res/color/color_state_list_refs34.xml
M appcompat/appcompat/src/androidTest/res/layout/appcompat_edittext_activity.xml
A appcompat/appcompat/src/androidTest/res/values/attrs.xml
M appcompat/appcompat/src/androidTest/res/values/strings.xml
M appcompat/appcompat/src/androidTest/res/values/styles.xml
M appcompat/appcompat/src/main/java/androidx/appcompat/widget/AppCompatTextHelper.java

https://android-review.googlesource.com/1302103

ki...@google.com <ki...@google.com> May 5, 2020 06:50PM

Marked as fixed.