2013/11/21

MultiSelectListPreference にバグがあり、勝手に選択解除されることがある

Android の MultiSelectListPreference、便利なのですが致命的なバグがあることがわかりました。
以下のような手順で操作をすると、勝手に選択が解除されます。
場合によっては Preference の内容も書き換えてしまいます。

この問題は少なくとも 4.1.1_r1 で直っています。

再現手順

以下のような MultiSelectListPreference を用意します

res/xml/pref.xml
<PreferenceScreen xmlns:android="http://schemas.android.com/apk/res/android">
    <MultiSelectListPreference
        android:key="data_list"
        android:entries="@array/pref_example_list_titles"
        android:entryValues="@array/pref_example_list_values"
        android:negativeButtonText="@android:string/cancel"
        android:positiveButtonText="@android:string/ok"
        android:title="@string/pref_title_messages" />
</PreferenceScreen>

これを表示する PreferenceActivity を用意し、起動。
ダイアログを表示。
「A」を選択して、OKをタップ。

PreferenceActivity に戻る。

もう一度 Preference をタップしてダイアログを表示。
この時は、まだ「A」が選択されたまま。
なにもせず、OK をタップ。

PreferenceActivity に戻る。

もう一度ダイアログ表示。
すると、「A」の選択が外れている。

この問題、3回もダイアログを表示しないと見えてこないので、あまり問題にならなそうですが、実は2回目にダイアログを表示した時点で、内部の不整合が起こっています。
そのため、むしろユーザが認識できない形でバグが入り込んでしまうので、結構危険な気がします。

原因

この現象の原因は、MultiSelectListPreference 内の mValuesmNewValues の扱いに問題があることです。
mValues は現在選択されている項目、mNewValues は新しく選択した項目で、OK を押した場合には値が更新され、Cancel を押した場合にはmNewValuesが破棄されるという構造になっています。

コードを追っていきましょう。
まず、「A」を選択して OK を押すと、onDialogClosed() が呼ばれます。
そして、その中で setValues() が呼ばれます。

MultiSelectListPreference (Android 4.0.1_r1)
    @Override
    protected void onDialogClosed(boolean positiveResult) {
        super.onDialogClosed(positiveResult);
        
        if (positiveResult && mPreferenceChanged) {
            final Set<string> values = mNewValues;
            if (callChangeListener(values)) {
                setValues(values);
            }
        }
        mPreferenceChanged = false;
    }

setValues() の中では引数valuesmValues に無条件で代入されています。
呼び出し元をもう一度見てみましょう。引数には mNewValues が入っています。
つまり、この時点で、mValuesmNewValues は同じインスタンスを示すことになります。
    public void setValues(Set<string> values) {
        mValues = values;
        
        persistStringSet(values);
    }

この後、もう一度ダイアログを表示すると onPrepareDialogBuilder() が呼ばれます。
表示自体は getSelectedItems() で取得した 「A」 が選択された状態で表示されますが、mNewValues.clear() によって、mValues も clear されてしまいます
当然、mNewValues.addAll(mValues) をしても mNewValues は空のまま。 表示と状態が合っていない状態が出来上がります。
    @Override
    protected void onPrepareDialogBuilder(Builder builder) {
        super.onPrepareDialogBuilder(builder);
        
        if (mEntries == null || mEntryValues == null) {
            throw new IllegalStateException(
                    "MultiSelectListPreference requires an entries array and " +
                    "an entryValues array.");
        }
        
        boolean[] checkedItems = getSelectedItems();
        builder.setMultiChoiceItems(mEntries, checkedItems,
                new DialogInterface.OnMultiChoiceClickListener() {
                    public void onClick(DialogInterface dialog, int which, boolean isChecked) {
                        if (isChecked) {
                            mPreferenceChanged |= mNewValues.add(mEntryValues[which].toString());
                        } else {
                            mPreferenceChanged |= mNewValues.remove(mEntryValues[which].toString());
                        }
                    }
                });
        mNewValues.clear();
        mNewValues.addAll(mValues);
    }

この後、OK を押すと、空の mNewValuesmValues にセットしに行くので、次に表示する時には選択が解除された状態になるというわけです。

解決策

この問題、少なくとも Android 4.1.1_r1 以降では以下のように修正されています。

MultiSelectListPreference (Android 4.1.1_r1)
    public void setValues(Set<string> values) {
        mValues.clear();
        mValues.addAll(values);

        persistStringSet(values);
    }

入力をそのまま代入するのではなく、addAll() を使うことで参照が同じになってしまうのを防いでいるわけです。
ターゲットが Android 4.1 以降であるならば、この問題をケアする必要はありません。

しかし、まだターゲットが Android 4.1 以降のみというケースは少ないはず。

そういう場合にどうしたら良いか考えてみました。
まず最初に、OnPreferenceChangeListenerOnPreferenceClickListener を使って回避する方法を思いついたのですが、どちらもタイミングが悪く、mNewValuesmValues の一致を防ぐことは出来ません。

で、結局、新たなクラスを作るしかないという結論にたどり着きました。
まず、以下のようなクラスを作成します。

public class WrapperMultiSelectListPreference extends MultiSelectListPreference {
    public WrapperMultiSelectListPreference(Context context) {
        super(context);
    }

    public WrapperMultiSelectListPreference(Context context, AttributeSet attrs) {
        super(context, attrs);
    }

    @Override
    public void setValues(Set<String> values) {
        super.setValues(new HashSet<String>(values));
    }
}

これを、以下のように使用すればOKです。
<PreferenceScreen xmlns:android="http://schemas.android.com/apk/res/android">
    <com.kokufu.android.lib.preference.WrapperMultiSelectListPreference
        android:key="data_list"
        android:entries="@array/pref_example_list_titles"
        android:entryValues="@array/pref_example_list_values"
        android:negativeButtonText="@android:string/cancel"
        android:positiveButtonText="@android:string/ok"
        android:title="@string/pref_title_messages" />
<PreferenceScreen/>

マイナーなバグに対応するために、新たなクラスを定義するのはちょっと気に入りませんが、世の中に出てしまったバグなので致し方ありません。

ちなみに、バージョン間のコードの差分を確認するには GrepCode を利用すると便利です。

0 件のコメント: