diff --git a/app/src/main/java/org/thoughtcrime/securesms/groups/ui/creategroup/details/AddGroupDetailsFragment.java b/app/src/main/java/org/thoughtcrime/securesms/groups/ui/creategroup/details/AddGroupDetailsFragment.java index 052a3f9de..36761af29 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/groups/ui/creategroup/details/AddGroupDetailsFragment.java +++ b/app/src/main/java/org/thoughtcrime/securesms/groups/ui/creategroup/details/AddGroupDetailsFragment.java @@ -27,6 +27,7 @@ import com.bumptech.glide.request.target.CustomTarget; import com.bumptech.glide.request.transition.Transition; import com.dd.CircularProgressButton; +import org.signal.core.util.EditTextUtil; import org.thoughtcrime.securesms.LoggingFragment; import org.thoughtcrime.securesms.R; import org.thoughtcrime.securesms.groups.ui.GroupMemberListView; @@ -104,6 +105,7 @@ public class AddGroupDetailsFragment extends LoggingFragment { avatar.setOnClickListener(v -> showAvatarSelectionBottomSheet()); members.setRecipientClickListener(this::handleRecipientClick); + EditTextUtil.addGraphemeClusterLimitFilter(name, FeatureFlags.getMaxGroupNameGraphemeLength()); name.addTextChangedListener(new AfterTextChanged(editable -> viewModel.setName(editable.toString()))); toolbar.setNavigationOnClickListener(unused -> callback.onNavigationButtonPressed()); create.setOnClickListener(v -> handleCreateClicked()); diff --git a/app/src/main/java/org/thoughtcrime/securesms/profiles/edit/EditProfileFragment.java b/app/src/main/java/org/thoughtcrime/securesms/profiles/edit/EditProfileFragment.java index 8fbcfc178..f38bf97e4 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/profiles/edit/EditProfileFragment.java +++ b/app/src/main/java/org/thoughtcrime/securesms/profiles/edit/EditProfileFragment.java @@ -27,6 +27,7 @@ import androidx.navigation.Navigation; import com.bumptech.glide.load.engine.DiskCacheStrategy; import com.dd.CircularProgressButton; +import org.signal.core.util.EditTextUtil; import org.signal.core.util.StreamUtil; import org.signal.core.util.logging.Log; import org.thoughtcrime.securesms.LoggingFragment; @@ -63,7 +64,6 @@ public class EditProfileFragment extends LoggingFragment { private static final String TAG = Log.tag(EditProfileFragment.class); private static final short REQUEST_CODE_SELECT_AVATAR = 31726; - private static final int MAX_GROUP_NAME_LENGTH = 32; private Toolbar toolbar; private View title; @@ -213,15 +213,12 @@ public class EditProfileFragment extends LoggingFragment { this.avatar.setOnClickListener(v -> startAvatarSelection()); - this.givenName.addTextChangedListener(new AfterTextChanged(s -> { - trimInPlace(s, isEditingGroup); - viewModel.setGivenName(s.toString()); - })); - view.findViewById(R.id.mms_group_hint) .setVisibility(isEditingGroup && groupId.isMms() ? View.VISIBLE : View.GONE); if (isEditingGroup) { + EditTextUtil.addGraphemeClusterLimitFilter(givenName, FeatureFlags.getMaxGroupNameGraphemeLength()); + givenName.addTextChangedListener(new AfterTextChanged(s -> viewModel.setGivenName(s.toString()))); givenName.setHint(R.string.EditProfileFragment__group_name); givenName.requestFocus(); toolbar.setTitle(R.string.EditProfileFragment__edit_group_name_and_photo); @@ -231,8 +228,12 @@ public class EditProfileFragment extends LoggingFragment { view.findViewById(R.id.description_text).setVisibility(View.GONE); view.findViewById(R.id.avatar_placeholder).setImageResource(R.drawable.ic_group_outline_40); } else { + this.givenName.addTextChangedListener(new AfterTextChanged(s -> { + trimInPlace(s); + viewModel.setGivenName(s.toString()); + })); this.familyName.addTextChangedListener(new AfterTextChanged(s -> { - trimInPlace(s, false); + trimInPlace(s); viewModel.setFamilyName(s.toString()); })); LearnMoreTextView descriptionText = view.findViewById(R.id.description_text); @@ -335,7 +336,7 @@ public class EditProfileFragment extends LoggingFragment { controller.onProfileNameUploadCompleted(); } - @RequiresApi(api = Build.VERSION_CODES.LOLLIPOP) + @RequiresApi(api = 21) private void handleFinishedLollipop() { int[] finishButtonLocation = new int[2]; int[] revealLocation = new int[2]; @@ -374,9 +375,8 @@ public class EditProfileFragment extends LoggingFragment { animation.start(); } - private static void trimInPlace(Editable s, boolean isGroup) { - int trimmedLength = isGroup ? StringUtil.trimToFit(s.toString(), MAX_GROUP_NAME_LENGTH).length() - : StringUtil.trimToFit(s.toString(), ProfileName.MAX_PART_LENGTH).length(); + private static void trimInPlace(Editable s) { + int trimmedLength = StringUtil.trimToFit(s.toString(), ProfileName.MAX_PART_LENGTH).length(); if (s.length() > trimmedLength) { s.delete(trimmedLength, s.length()); diff --git a/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java b/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java index c3a2931ca..78e3c92ad 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java +++ b/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java @@ -14,7 +14,6 @@ import org.signal.core.util.logging.Log; import org.thoughtcrime.securesms.BuildConfig; import org.thoughtcrime.securesms.dependencies.ApplicationDependencies; import org.thoughtcrime.securesms.groups.SelectionLimits; -import org.thoughtcrime.securesms.jobs.RefreshAttributesJob; import org.thoughtcrime.securesms.jobs.RemoteConfigRefreshJob; import org.thoughtcrime.securesms.keyvalue.SignalStore; @@ -53,6 +52,7 @@ public final class FeatureFlags { private static final String USERNAMES = "android.usernames"; private static final String GROUPS_V2_RECOMMENDED_LIMIT = "global.groupsv2.maxGroupSize"; private static final String GROUPS_V2_HARD_LIMIT = "global.groupsv2.groupSizeHardLimit"; + private static final String GROUP_NAME_MAX_LENGTH = "global.groupsv2.maxNameLength"; private static final String INTERNAL_USER = "android.internalUser"; private static final String VERIFY_V2 = "android.verifyV2"; private static final String PHONE_NUMBER_PRIVACY_VERSION = "android.phoneNumberPrivacyVersion"; @@ -89,7 +89,8 @@ public final class FeatureFlags { GROUP_CALLING, SEND_VIEWED_RECEIPTS, CUSTOM_VIDEO_MUXER, - CDS_REFRESH_INTERVAL + CDS_REFRESH_INTERVAL, + GROUP_NAME_MAX_LENGTH ); @VisibleForTesting @@ -122,7 +123,8 @@ public final class FeatureFlags { GROUP_CALLING, GV1_MIGRATION_JOB, CUSTOM_VIDEO_MUXER, - CDS_REFRESH_INTERVAL + CDS_REFRESH_INTERVAL, + GROUP_NAME_MAX_LENGTH ); /** @@ -278,6 +280,11 @@ public final class FeatureFlags { return getInteger(CDS_REFRESH_INTERVAL, (int) TimeUnit.HOURS.toSeconds(48)); } + /** The maximum number of grapheme */ + public static int getMaxGroupNameGraphemeLength() { + return Math.max(32, getInteger(GROUP_NAME_MAX_LENGTH, -1)); + } + /** Only for rendering debug info. */ public static synchronized @NonNull Map getMemoryValues() { return new TreeMap<>(REMOTE_VALUES); diff --git a/app/src/main/res/layout/add_group_details_fragment.xml b/app/src/main/res/layout/add_group_details_fragment.xml index ae15ea8c2..00c1b21af 100644 --- a/app/src/main/res/layout/add_group_details_fragment.xml +++ b/app/src/main/res/layout/add_group_details_fragment.xml @@ -33,7 +33,6 @@ android:background="@null" android:hint="@string/AddGroupDetailsFragment__group_name_required" android:inputType="text" - android:maxLength="34" android:maxLines="1" app:layout_constraintBottom_toBottomOf="@id/group_avatar" app:layout_constraintEnd_toEndOf="parent" diff --git a/core-util/src/main/java/org/signal/core/util/BreakIteratorCompat.java b/core-util/src/main/java/org/signal/core/util/BreakIteratorCompat.java new file mode 100644 index 000000000..f0e28acbc --- /dev/null +++ b/core-util/src/main/java/org/signal/core/util/BreakIteratorCompat.java @@ -0,0 +1,146 @@ +package org.signal.core.util; + +import android.os.Build; + +import androidx.annotation.NonNull; +import androidx.annotation.RequiresApi; + +import java.util.Iterator; + +public abstract class BreakIteratorCompat implements Iterable { + public static final int DONE = -1; + private CharSequence charSequence; + + public abstract int first(); + + public abstract int next(); + + public void setText(CharSequence charSequence) { + this.charSequence = charSequence; + } + + public static BreakIteratorCompat getInstance() { + if (Build.VERSION.SDK_INT >= 24) { + return new AndroidIcuBreakIterator(); + } else { + return new FallbackBreakIterator(); + } + } + + public int countBreaks() { + int breakCount = 0; + + first(); + + while (next() != DONE) { + breakCount++; + } + + return breakCount; + } + + @Override + public @NonNull Iterator iterator() { + return new Iterator() { + + int index1 = BreakIteratorCompat.this.first(); + int index2 = BreakIteratorCompat.this.next(); + + @Override + public boolean hasNext() { + return index2 != DONE; + } + + @Override + public CharSequence next() { + CharSequence c = index2 != DONE ? charSequence.subSequence(index1, index2) : ""; + + index1 = index2; + index2 = BreakIteratorCompat.this.next(); + + return c; + } + }; + } + + /** + * Take {@param atMost} graphemes from the start of string. + */ + public final CharSequence take(int atMost) { + if (atMost <= 0) return ""; + + StringBuilder stringBuilder = new StringBuilder(charSequence.length()); + int count = 0; + + for (CharSequence grapheme : this) { + stringBuilder.append(grapheme); + + count++; + + if (count >= atMost) break; + } + + return stringBuilder.toString(); + } + + /** + * An BreakIteratorCompat implementation that delegates calls to `android.icu.text.BreakIterator`. + * This class handles grapheme clusters fine but requires Android API >= 24. + */ + @RequiresApi(24) + private static class AndroidIcuBreakIterator extends BreakIteratorCompat { + private final android.icu.text.BreakIterator breakIterator = android.icu.text.BreakIterator.getCharacterInstance(); + + @Override + public int first() { + return breakIterator.first(); + } + + @Override + public int next() { + return breakIterator.next(); + } + + @Override + public void setText(CharSequence charSequence) { + super.setText(charSequence); + if (Build.VERSION.SDK_INT >= 29) { + breakIterator.setText(charSequence); + } else { + breakIterator.setText(charSequence.toString()); + } + } + } + + /** + * An BreakIteratorCompat implementation that delegates calls to `java.text.BreakIterator`. + * This class may or may not handle grapheme clusters well depending on the underlying implementation. + * In the emulator, API 23 implements ICU version of the BreakIterator so that it handles grapheme + * clusters fine. But API 21 implements RuleBasedIterator which does not handle grapheme clusters. + *

+ * If it doesn't handle grapheme clusters correctly, in most cases the combined characters are + * broken up into pieces when the code tries to trim a string. For example, an emoji that is + * a combination of a person, gender and skin tone, trimming the character using this class may result + * in trimming the parts of the character, e.g. a dark skin frowning woman emoji may result in + * a neutral skin frowning woman emoji. + */ + private static class FallbackBreakIterator extends BreakIteratorCompat { + private final java.text.BreakIterator breakIterator = java.text.BreakIterator.getCharacterInstance(); + + @Override + public int first() { + return breakIterator.first(); + } + + @Override + public int next() { + return breakIterator.next(); + } + + @Override + public void setText(CharSequence charSequence) { + super.setText(charSequence); + breakIterator.setText(charSequence.toString()); + } + } +} diff --git a/core-util/src/main/java/org/signal/core/util/EditTextUtil.java b/core-util/src/main/java/org/signal/core/util/EditTextUtil.java new file mode 100644 index 000000000..ce820aa6d --- /dev/null +++ b/core-util/src/main/java/org/signal/core/util/EditTextUtil.java @@ -0,0 +1,20 @@ +package org.signal.core.util; + +import android.text.InputFilter; +import android.widget.EditText; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +public final class EditTextUtil { + + private EditTextUtil() { + } + + public static void addGraphemeClusterLimitFilter(EditText text, int maximumGraphemes) { + List filters = new ArrayList<>(Arrays.asList(text.getFilters())); + filters.add(new GraphemeClusterLimitFilter(maximumGraphemes)); + text.setFilters(filters.toArray(new InputFilter[0])); + } +} diff --git a/core-util/src/main/java/org/signal/core/util/GraphemeClusterLimitFilter.java b/core-util/src/main/java/org/signal/core/util/GraphemeClusterLimitFilter.java new file mode 100644 index 000000000..37dafeec3 --- /dev/null +++ b/core-util/src/main/java/org/signal/core/util/GraphemeClusterLimitFilter.java @@ -0,0 +1,53 @@ +package org.signal.core.util; + +import android.text.InputFilter; +import android.text.Spanned; + +import org.signal.core.util.logging.Log; + +/** + * This filter will constrain edits not to make the number of character breaks of the text + * greater than the specified maximum. + *

+ * This means it will limit to a maximum number of grapheme clusters. + */ +public final class GraphemeClusterLimitFilter implements InputFilter { + + private static final String TAG = Log.tag(GraphemeClusterLimitFilter.class); + + private final BreakIteratorCompat breakIteratorCompat; + private final int max; + + public GraphemeClusterLimitFilter(int max) { + this.breakIteratorCompat = BreakIteratorCompat.getInstance(); + this.max = max; + } + + @Override + public CharSequence filter(CharSequence source, int start, int end, Spanned dest, int dstart, int dend) { + CharSequence sourceFragment = source.subSequence(start, end); + + CharSequence head = dest.subSequence(0, dstart); + CharSequence tail = dest.subSequence(dend, dest.length()); + + breakIteratorCompat.setText(String.format("%s%s%s", head, sourceFragment, tail)); + int length = breakIteratorCompat.countBreaks(); + + if (length > max) { + breakIteratorCompat.setText(sourceFragment); + int sourceLength = breakIteratorCompat.countBreaks(); + CharSequence trimmedSource = breakIteratorCompat.take(sourceLength - (length - max)); + + breakIteratorCompat.setText(String.format("%s%s%s", head, trimmedSource, tail)); + int newExpectedCount = breakIteratorCompat.countBreaks(); + if (newExpectedCount > max) { + Log.w(TAG, "Failed to create string under the required length " + newExpectedCount); + return ""; + } + + return trimmedSource; + } + + return source; + } +} diff --git a/core-util/src/test/java/org/signal/core/util/BreakIteratorCompatTest.java b/core-util/src/test/java/org/signal/core/util/BreakIteratorCompatTest.java new file mode 100644 index 000000000..1d5dda358 --- /dev/null +++ b/core-util/src/test/java/org/signal/core/util/BreakIteratorCompatTest.java @@ -0,0 +1,104 @@ +package org.signal.core.util; + +import org.junit.Test; + +import java.util.ArrayList; +import java.util.List; + +import static java.util.Arrays.asList; +import static org.junit.Assert.assertEquals; + +public final class BreakIteratorCompatTest { + + @Test + public void empty() { + BreakIteratorCompat breakIterator = BreakIteratorCompat.getInstance(); + breakIterator.setText(""); + + assertEquals(BreakIteratorCompat.DONE, breakIterator.next()); + } + + @Test + public void single() { + BreakIteratorCompat breakIterator = BreakIteratorCompat.getInstance(); + breakIterator.setText("a"); + + assertEquals(1, breakIterator.next()); + assertEquals(BreakIteratorCompat.DONE, breakIterator.next()); + } + + @Test + public void count_empty() { + BreakIteratorCompat breakIterator = BreakIteratorCompat.getInstance(); + breakIterator.setText(""); + + assertEquals(0, breakIterator.countBreaks()); + assertEquals(BreakIteratorCompat.DONE, breakIterator.next()); + } + + @Test + public void count_simple_text() { + BreakIteratorCompat breakIterator = BreakIteratorCompat.getInstance(); + breakIterator.setText("abc"); + + assertEquals(3, breakIterator.countBreaks()); + assertEquals(BreakIteratorCompat.DONE, breakIterator.next()); + } + + @Test + public void two_counts() { + BreakIteratorCompat breakIterator = BreakIteratorCompat.getInstance(); + breakIterator.setText("abc"); + + assertEquals(3, breakIterator.countBreaks()); + assertEquals(BreakIteratorCompat.DONE, breakIterator.next()); + assertEquals(3, breakIterator.countBreaks()); + } + + @Test + public void count_multi_character_graphemes() { + String hindi = "समाजो गयेग"; + + BreakIteratorCompat breakIterator = BreakIteratorCompat.getInstance(); + breakIterator.setText(hindi); + + assertEquals(7, breakIterator.countBreaks()); + assertEquals(BreakIteratorCompat.DONE, breakIterator.next()); + } + + @Test + public void iterate_multi_character_graphemes() { + String hindi = "समाजो गयेग"; + + BreakIteratorCompat breakIterator = BreakIteratorCompat.getInstance(); + breakIterator.setText(hindi); + + assertEquals(asList("स", "मा", "जो", " ", "ग", "ये", "ग"), toList(breakIterator)); + assertEquals(BreakIteratorCompat.DONE, breakIterator.next()); + } + + @Test + public void split_multi_character_graphemes() { + String hindi = "समाजो गयेग"; + + BreakIteratorCompat breakIterator = BreakIteratorCompat.getInstance(); + breakIterator.setText(hindi); + + assertEquals("समाजो गयेग", breakIterator.take(8)); + assertEquals("समाजो गयेग", breakIterator.take(7)); + assertEquals("समाजो गये", breakIterator.take(6)); + assertEquals("समाजो ग", breakIterator.take(5)); + assertEquals("समाजो ", breakIterator.take(4)); + assertEquals("समाजो", breakIterator.take(3)); + assertEquals("समा", breakIterator.take(2)); + assertEquals("स", breakIterator.take(1)); + assertEquals("", breakIterator.take(0)); + assertEquals("", breakIterator.take(-1)); + } + + private List toList(BreakIteratorCompat breakIterator) { + List list = new ArrayList<>(); + breakIterator.forEach(list::add); + return list; + } +}