From 2170e6b0fa90c24cb8297e63ecf65688aa2180fc Mon Sep 17 00:00:00 2001 From: Levi Bard Date: Wed, 4 Nov 2020 18:21:41 +0100 Subject: [PATCH] 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 --- .../tusky/adapter/FollowRequestViewHolder.kt | 19 ++++++++--- .../tusky/adapter/FollowRequestsAdapter.java | 2 +- .../tusky/adapter/NotificationsAdapter.java | 18 +++++----- .../notifications/NotificationHelper.java | 33 +++++++++---------- .../keylesspalace/tusky/util/StringUtils.kt | 9 +++++ .../item_follow_request_notification.xml | 2 +- 6 files changed, 49 insertions(+), 34 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/adapter/FollowRequestViewHolder.kt b/app/src/main/java/com/keylesspalace/tusky/adapter/FollowRequestViewHolder.kt index ff87f9781..dec4586ba 100644 --- 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) diff --git a/app/src/main/java/com/keylesspalace/tusky/adapter/FollowRequestsAdapter.java b/app/src/main/java/com/keylesspalace/tusky/adapter/FollowRequestsAdapter.java index 548d893c5..dab3d4fe3 100644 --- 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); } } diff --git a/app/src/main/java/com/keylesspalace/tusky/adapter/NotificationsAdapter.java b/app/src/main/java/com/keylesspalace/tusky/adapter/NotificationsAdapter.java index 936d0edf1..dcaf41690 100644 --- 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 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(); diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationHelper.java b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationHelper.java index a094750e6..3020f2071 100644 --- 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), diff --git a/app/src/main/java/com/keylesspalace/tusky/util/StringUtils.kt b/app/src/main/java/com/keylesspalace/tusky/util/StringUtils.kt index c7af0ca69..83eaeafad 100644 --- 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" +} \ No newline at end of file diff --git a/app/src/main/res/layout/item_follow_request_notification.xml b/app/src/main/res/layout/item_follow_request_notification.xml index 712c08d42..d4db2a3dd 100644 --- 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"