Perform bidirectionality isolation manually instead of relying on BidiFormatter (#1976)

* Perform manual isolation of display names etc. instead of relying on BidiFormatter. Fixes #1921 * Make follow request notification header formatting more like other notifications
2020-11-04 18:21:41 +01:00 · 2020-11-04 18:21:41 +01:00 · 2170e6b0fa
commit 2170e6b0fa
parent d7e47caef3
6 changed files with 49 additions and 34 deletions
--- a/app/src/main/java/com/keylesspalace/tusky/adapter/FollowRequestViewHolder.kt
+++ b/app/src/main/java/com/keylesspalace/tusky/adapter/FollowRequestViewHolder.kt
@ -1,13 +1,19 @@
 package com.keylesspalace.tusky.adapter

+import android.graphics.Typeface
+import android.text.SpannableStringBuilder
+import android.text.Spanned
+import android.text.style.StyleSpan
 import android.view.View
-import androidx.core.text.BidiFormatter
 import androidx.preference.PreferenceManager
 import androidx.recyclerview.widget.RecyclerView
 import com.keylesspalace.tusky.R
 import com.keylesspalace.tusky.entity.Account
 import com.keylesspalace.tusky.interfaces.AccountActionListener
-import com.keylesspalace.tusky.util.*
+import com.keylesspalace.tusky.util.emojify
+import com.keylesspalace.tusky.util.loadAvatar
+import com.keylesspalace.tusky.util.unicodeWrap
+import com.keylesspalace.tusky.util.visible
 import kotlinx.android.synthetic.main.item_follow_request_notification.view.*

 internal class FollowRequestViewHolder(itemView: View, private val showHeader: Boolean) : RecyclerView.ViewHolder(itemView) {
@ -15,13 +21,16 @@ internal class FollowRequestViewHolder(itemView: View, private val showHeader: B
    private val animateAvatar: Boolean = PreferenceManager.getDefaultSharedPreferences(itemView.context)
            .getBoolean("animateGifAvatars", false)

-    fun setupWithAccount(account: Account, formatter: BidiFormatter?) {
+    fun setupWithAccount(account: Account) {
        id = account.id
-        val wrappedName = formatter?.unicodeWrap(account.name) ?: account.name
+        val wrappedName = account.name.unicodeWrap()
        val emojifiedName: CharSequence = wrappedName.emojify(account.emojis, itemView)
        itemView.displayNameTextView.text = emojifiedName
        if (showHeader) {
-            itemView.notificationTextView?.text = itemView.context.getString(R.string.notification_follow_request_format, emojifiedName)
+            val wholeMessage: String = itemView.context.getString(R.string.notification_follow_request_format, wrappedName)
+            itemView.notificationTextView?.text = SpannableStringBuilder(wholeMessage).apply {
+                setSpan(StyleSpan(Typeface.BOLD), 0, wrappedName.length, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE)
+            }.emojify(account.emojis, itemView)
        }
        itemView.notificationTextView?.visible(showHeader)
        val format = itemView.context.getString(R.string.status_username_format)
--- a/app/src/main/java/com/keylesspalace/tusky/adapter/FollowRequestsAdapter.java
+++ b/app/src/main/java/com/keylesspalace/tusky/adapter/FollowRequestsAdapter.java
@ -53,7 +53,7 @@ public class FollowRequestsAdapter extends AccountAdapter {
    public void onBindViewHolder(@NonNull RecyclerView.ViewHolder viewHolder, int position) {
        if (getItemViewType(position) == VIEW_TYPE_ACCOUNT) {
            FollowRequestViewHolder holder = (FollowRequestViewHolder) viewHolder;
-            holder.setupWithAccount(accountList.get(position), null);
+            holder.setupWithAccount(accountList.get(position));
            holder.setupActionListener(accountActionListener);
        }
    }
--- a/app/src/main/java/com/keylesspalace/tusky/adapter/NotificationsAdapter.java
+++ b/app/src/main/java/com/keylesspalace/tusky/adapter/NotificationsAdapter.java
@ -35,7 +35,6 @@ import android.widget.TextView;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.core.content.ContextCompat;
-import androidx.core.text.BidiFormatter;
 import androidx.recyclerview.widget.RecyclerView;

 import com.keylesspalace.tusky.R;
@ -51,6 +50,7 @@ import com.keylesspalace.tusky.util.ImageLoadingHelper;
 import com.keylesspalace.tusky.util.LinkHelper;
 import com.keylesspalace.tusky.util.SmartLengthInputFilter;
 import com.keylesspalace.tusky.util.StatusDisplayOptions;
+import com.keylesspalace.tusky.util.StringUtils;
 import com.keylesspalace.tusky.util.TimestampUtils;
 import com.keylesspalace.tusky.viewdata.NotificationViewData;
 import com.keylesspalace.tusky.viewdata.StatusViewData;
@ -86,7 +86,6 @@ public class NotificationsAdapter extends RecyclerView.Adapter {
    private StatusActionListener statusListener;
    private NotificationActionListener notificationActionListener;
    private AccountActionListener accountActionListener;
-    private BidiFormatter bidiFormatter;
    private AdapterDataSource<NotificationViewData> dataSource;

    public NotificationsAdapter(String accountId,
@ -102,7 +101,6 @@ public class NotificationsAdapter extends RecyclerView.Adapter {
        this.statusListener = statusListener;
        this.notificationActionListener = notificationActionListener;
        this.accountActionListener = accountActionListener;
-        bidiFormatter = BidiFormatter.getInstance();
    }

    @NonNull
@ -204,7 +202,7 @@ public class NotificationsAdapter extends RecyclerView.Adapter {
                                    concreteNotificaton.getAccount().getAvatar());
                        }

-                        holder.setMessage(concreteNotificaton, statusListener, bidiFormatter);
+                        holder.setMessage(concreteNotificaton, statusListener);
                        holder.setupButtons(notificationActionListener,
                                concreteNotificaton.getAccount().getId(),
                                concreteNotificaton.getId());
@ -221,7 +219,7 @@ public class NotificationsAdapter extends RecyclerView.Adapter {
                case VIEW_TYPE_FOLLOW: {
                    if (payloadForHolder == null) {
                        FollowViewHolder holder = (FollowViewHolder) viewHolder;
-                        holder.setMessage(concreteNotificaton.getAccount(), bidiFormatter);
+                        holder.setMessage(concreteNotificaton.getAccount());
                        holder.setupButtons(notificationActionListener, concreteNotificaton.getAccount().getId());
                    }
                    break;
@ -229,7 +227,7 @@ public class NotificationsAdapter extends RecyclerView.Adapter {
                case VIEW_TYPE_FOLLOW_REQUEST: {
                    if (payloadForHolder == null) {
                        FollowRequestViewHolder holder = (FollowRequestViewHolder) viewHolder;
-                        holder.setupWithAccount(concreteNotificaton.getAccount(), bidiFormatter);
+                        holder.setupWithAccount(concreteNotificaton.getAccount());
                        holder.setupActionListener(accountActionListener);
                    }
                }
@ -325,11 +323,11 @@ public class NotificationsAdapter extends RecyclerView.Adapter {
            this.statusDisplayOptions = statusDisplayOptions;
        }

-        void setMessage(Account account, BidiFormatter bidiFormatter) {
+        void setMessage(Account account) {
            Context context = message.getContext();

            String format = context.getString(R.string.notification_follow_format);
-            String wrappedDisplayName = bidiFormatter.unicodeWrap(account.getName());
+            String wrappedDisplayName = StringUtils.unicodeWrap(account.getName());
            String wholeMessage = String.format(format, wrappedDisplayName);
            CharSequence emojifiedMessage = CustomEmojiHelper.emojify(wholeMessage, account.getEmojis(), message);
            message.setText(emojifiedMessage);
@ -459,10 +457,10 @@ public class NotificationsAdapter extends RecyclerView.Adapter {
            }
        }

-        void setMessage(NotificationViewData.Concrete notificationViewData, LinkListener listener, BidiFormatter bidiFormatter) {
+        void setMessage(NotificationViewData.Concrete notificationViewData, LinkListener listener) {
            this.statusViewData = notificationViewData.getStatusViewData();

-            String displayName = bidiFormatter.unicodeWrap(notificationViewData.getAccount().getName());
+            String displayName = StringUtils.unicodeWrap(notificationViewData.getAccount().getName());
            Notification.Type type = notificationViewData.getType();

            Context context = message.getContext();
--- a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationHelper.java
+++ b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationHelper.java
@ -37,7 +37,6 @@ import androidx.core.app.NotificationManagerCompat;
 import androidx.core.app.RemoteInput;
 import androidx.core.app.TaskStackBuilder;
 import androidx.core.content.ContextCompat;
-import androidx.core.text.BidiFormatter;
 import androidx.work.Constraints;
 import androidx.work.NetworkType;
 import androidx.work.PeriodicWorkRequest;
@ -58,6 +57,7 @@ import com.keylesspalace.tusky.entity.PollOption;
 import com.keylesspalace.tusky.entity.Status;
 import com.keylesspalace.tusky.receiver.NotificationClearBroadcastReceiver;
 import com.keylesspalace.tusky.receiver.SendStatusBroadcastReceiver;
+import com.keylesspalace.tusky.util.StringUtils;
 import com.keylesspalace.tusky.viewdata.PollViewDataKt;

 import org.json.JSONArray;
@ -145,7 +145,6 @@ public class NotificationHelper {

        String rawCurrentNotifications = account.getActiveNotifications();
        JSONArray currentNotifications;
-        BidiFormatter bidiFormatter = BidiFormatter.getInstance();

        try {
            currentNotifications = new JSONArray(rawCurrentNotifications);
@ -174,7 +173,7 @@ public class NotificationHelper {

        notificationId++;

-        builder.setContentTitle(titleForType(context, body, bidiFormatter, account))
+        builder.setContentTitle(titleForType(context, body, account))
                .setContentText(bodyForType(body, context));

        if (body.getType() == Notification.Type.MENTION || body.getType() == Notification.Type.POLL) {
@ -243,7 +242,7 @@ public class NotificationHelper {
        if (currentNotifications.length() != 1) {
            try {
                String title = context.getString(R.string.notification_title_summary, currentNotifications.length());
-                String text = joinNames(context, currentNotifications, bidiFormatter);
+                String text = joinNames(context, currentNotifications);
                summaryBuilder.setContentTitle(title)
                        .setContentText(text);
            } catch (JSONException e) {
@ -573,36 +572,36 @@ public class NotificationHelper {
        }
    }

-    private static String wrapItemAt(JSONArray array, int index, BidiFormatter bidiFormatter) throws JSONException {
-        return bidiFormatter.unicodeWrap(array.get(index).toString());
+    private static String wrapItemAt(JSONArray array, int index) throws JSONException {
+        return StringUtils.unicodeWrap(array.get(index).toString());
    }

    @Nullable
-    private static String joinNames(Context context, JSONArray array, BidiFormatter bidiFormatter) throws JSONException {
+    private static String joinNames(Context context, JSONArray array) throws JSONException {
        if (array.length() > 3) {
            int length = array.length();
            return String.format(context.getString(R.string.notification_summary_large),
-                    wrapItemAt(array, length - 1, bidiFormatter),
-                    wrapItemAt(array, length - 2, bidiFormatter),
-                    wrapItemAt(array, length - 3, bidiFormatter),
+                    wrapItemAt(array, length - 1),
+                    wrapItemAt(array, length - 2),
+                    wrapItemAt(array, length - 3),
                    length - 3);
        } else if (array.length() == 3) {
            return String.format(context.getString(R.string.notification_summary_medium),
-                    wrapItemAt(array, 2, bidiFormatter),
-                    wrapItemAt(array, 1, bidiFormatter),
-                    wrapItemAt(array, 0, bidiFormatter));
+                    wrapItemAt(array, 2),
+                    wrapItemAt(array, 1),
+                    wrapItemAt(array, 0));
        } else if (array.length() == 2) {
            return String.format(context.getString(R.string.notification_summary_small),
-                    wrapItemAt(array, 1, bidiFormatter),
-                    wrapItemAt(array, 0, bidiFormatter));
+                    wrapItemAt(array, 1),
+                    wrapItemAt(array, 0));
        }

        return null;
    }

    @Nullable
-    private static String titleForType(Context context, Notification notification, BidiFormatter bidiFormatter, AccountEntity account) {
-        String accountName = bidiFormatter.unicodeWrap(notification.getAccount().getName());
+    private static String titleForType(Context context, Notification notification, AccountEntity account) {
+        String accountName = StringUtils.unicodeWrap(notification.getAccount().getName());
        switch (notification.getType()) {
            case MENTION:
                return String.format(context.getString(R.string.notification_mention_format),
--- a/app/src/main/java/com/keylesspalace/tusky/util/StringUtils.kt
+++ b/app/src/main/java/com/keylesspalace/tusky/util/StringUtils.kt
@ -80,3 +80,12 @@ fun Spanned.trimTrailingWhitespace(): Spanned {
    } while (i >= 0 && get(i).isWhitespace())
    return subSequence(0, i + 1) as Spanned
 }
+
+/**
+ * BidiFormatter.unicodeWrap is insufficient in some cases (see #1921)
+ * So we force isolation manually
+ * https://unicode.org/reports/tr9/#Explicit_Directional_Isolates
+ */
+fun CharSequence.unicodeWrap(): String {
+    return "\u2068${this}\u2069"
+}
--- a/app/src/main/res/layout/item_follow_request_notification.xml
+++ b/app/src/main/res/layout/item_follow_request_notification.xml
@ -20,7 +20,7 @@
        android:gravity="center_vertical"
        android:maxLines="1"
        android:paddingStart="28dp"
-        android:textColor="?android:textColorTertiary"
+        android:textColor="?android:textColorSecondary"
        android:textSize="?attr/status_text_medium"
        app:layout_constraintStart_toStartOf="parent"
        app:layout_constraintTop_toTopOf="parent"