Skip to content

Commit

Permalink
Trimming profile names to fit byte budget and remove leading/trailing…
Browse files Browse the repository at this point in the history 
… spaces.
  • Loading branch information
@alan-signal @greyson-signal
alan-signal authored and greyson-signal committed Jan 27, 2020
1 parent 7d15c60 commit e7f568e
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 52 deletions.
25 changes: 19 additions & 6 deletions app/src/main/java/org/thoughtcrime/securesms/profiles/ProfileName.java
View file
Expand Up @@ -5,6 +5,7 @@

import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;

import com.annimon.stream.Stream;

Expand All @@ -24,8 +25,8 @@ public final class ProfileName implements Parcelable {
private final String joinedName;

private ProfileName(@Nullable String givenName, @Nullable String familyName) {
this.givenName = sanitize(givenName);
this.familyName = sanitize(familyName);
this.givenName = givenName == null ? "" : givenName;
this.familyName = familyName == null ? "" : familyName;
this.joinedName = getJoinedName(this.givenName, this.familyName);
}

Expand All @@ -43,16 +44,21 @@ String getFamilyName() {
return familyName;
}

public boolean isProfileNameCJKV() {
@VisibleForTesting
boolean isProfileNameCJKV() {
return isCJKV(givenName, familyName);
}

public boolean isEmpty() {
return joinedName.isEmpty();
}

public boolean isGivenNameEmpty() {
return givenName.isEmpty();
}

public @NonNull String serialize() {
if (isEmpty()) {
if (isGivenNameEmpty()) {
return "";
}

Expand Down Expand Up @@ -87,12 +93,19 @@ public boolean isEmpty() {
* Creates a profile name, trimming chars until it fits the limits.
*/
public static @NonNull ProfileName fromParts(@Nullable String givenName, @Nullable String familyName) {
if (givenName == null || givenName.isEmpty()) return EMPTY;
givenName = givenName == null ? "" : givenName;
familyName = familyName == null ? "" : familyName;

givenName = trimToFit(givenName .trim());
familyName = trimToFit(familyName.trim());

return new ProfileName(givenName, familyName);
}

private static @NonNull String sanitize(@Nullable String name) {
/**
* Trims a name string to fit into the byte length requirement.
*/
public static @NonNull String trimToFit(@Nullable String name) {
if (name == null) return "";

// At least one byte per char, so shorten string to reduce loop
Expand Down
80 changes: 45 additions & 35 deletions app/src/main/java/org/thoughtcrime/securesms/profiles/edit/EditProfileFragment.java
View file
Expand Up @@ -2,15 +2,13 @@

import android.Manifest;
import android.animation.Animator;
import android.annotation.SuppressLint;
import android.app.Activity;
import android.content.Context;
import android.content.Intent;
import android.net.Uri;
import android.os.AsyncTask;
import android.os.Build;
import android.os.Bundle;
import android.text.Selection;
import android.text.Editable;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewAnimationUtils;
Expand All @@ -26,7 +24,6 @@
import androidx.annotation.RequiresApi;
import androidx.annotation.StringRes;
import androidx.fragment.app.Fragment;
import androidx.lifecycle.Observer;
import androidx.lifecycle.ViewModelProviders;
import androidx.navigation.NavDirections;
import androidx.navigation.Navigation;
Expand All @@ -45,6 +42,7 @@
import org.thoughtcrime.securesms.util.BitmapDecodingException;
import org.thoughtcrime.securesms.util.BitmapUtil;
import org.thoughtcrime.securesms.util.FeatureFlags;
import org.thoughtcrime.securesms.util.concurrent.SimpleTask;
import org.thoughtcrime.securesms.util.text.AfterTextChanged;
import org.whispersystems.libsignal.util.guava.Optional;

Expand All @@ -70,7 +68,7 @@ public class EditProfileFragment extends Fragment {
private TextView username;

private Intent nextIntent;
private File captureFile;
private File captureFile;

private EditProfileViewModel viewModel;

Expand Down Expand Up @@ -113,7 +111,7 @@ public View onCreateView(@NonNull LayoutInflater inflater, @Nullable ViewGroup c
@Override
public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceState) {
initializeResources(view);
initializeViewModel(getArguments().getBoolean(EXCLUDE_SYSTEM, false));
initializeViewModel(requireArguments().getBoolean(EXCLUDE_SYSTEM, false));
initializeProfileName();
initializeProfileAvatar();
initializeUsername();
Expand All @@ -122,7 +120,7 @@ public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceStat
}

@Override
public void onRequestPermissionsResult(int requestCode, @NonNull String permissions[], @NonNull int[] grantResults) {
public void onRequestPermissionsResult(int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) {
Permissions.onRequestPermissionsResult(this, requestCode, permissions, grantResults);
}

Expand Down Expand Up @@ -151,24 +149,20 @@ public void onActivityResult(int requestCode, int resultCode, Intent data) {
break;
case AvatarSelection.REQUEST_CODE_CROP_IMAGE:
if (resultCode == Activity.RESULT_OK) {
new AsyncTask<Void, Void, byte[]>() {
@Override
protected byte[] doInBackground(Void... params) {
SimpleTask.run(() -> {
try {
BitmapUtil.ScaleResult result = BitmapUtil.createScaledBytes(requireActivity(), AvatarSelection.getResultUri(data), new ProfileMediaConstraints());
return result.getBitmap();
} catch (BitmapDecodingException e) {
Log.w(TAG, e);
return null;
}
}

@Override
protected void onPostExecute(byte[] result) {
if (result != null) {
viewModel.setAvatar(result);
},
(avatarBytes) -> {
if (avatarBytes != null) {
viewModel.setAvatar(avatarBytes);
GlideApp.with(EditProfileFragment.this)
.load(result)
.load(avatarBytes)
.skipMemoryCache(true)
.diskCacheStrategy(DiskCacheStrategy.NONE)
.circleCrop()
Expand All @@ -177,7 +171,7 @@ protected void onPostExecute(byte[] result) {
Toast.makeText(requireActivity(), R.string.CreateProfileActivity_error_setting_profile_photo, Toast.LENGTH_LONG).show();
}
}
}.execute();
);
}
break;
}
Expand All @@ -191,6 +185,7 @@ private void initializeViewModel(boolean excludeSystem) {
}

private void initializeResources(@NonNull View view) {
Bundle arguments = requireArguments();

this.avatar = view.findViewById(R.id.avatar);
this.givenName = view.findViewById(R.id.given_name);
Expand All @@ -201,9 +196,9 @@ private void initializeResources(@NonNull View view) {
this.username = view.findViewById(R.id.profile_overview_username);
this.usernameEditButton = view.findViewById(R.id.profile_overview_username_edit_button);
this.usernameLabel = view.findViewById(R.id.profile_overview_username_label);
this.nextIntent = getArguments().getParcelable(NEXT_INTENT);
this.nextIntent = arguments.getParcelable(NEXT_INTENT);

if (FeatureFlags.usernames() && getArguments().getBoolean(DISPLAY_USERNAME, false)) {
if (FeatureFlags.usernames() && arguments.getBoolean(DISPLAY_USERNAME, false)) {
username.setVisibility(View.VISIBLE);
usernameEditButton.setVisibility(View.VISIBLE);
usernameLabel.setVisibility(View.VISIBLE);
Expand All @@ -215,16 +210,22 @@ private void initializeResources(@NonNull View view) {
.onAnyResult(this::startAvatarSelection)
.execute());

this.givenName.addTextChangedListener(new AfterTextChanged(s -> viewModel.setGivenName(s.toString())));
this.familyName.addTextChangedListener(new AfterTextChanged(s -> viewModel.setFamilyName(s.toString())));
this.givenName .addTextChangedListener(new AfterTextChanged(s -> {
trimInPlace(s);
viewModel.setGivenName(s.toString());
}));
this.familyName.addTextChangedListener(new AfterTextChanged(s -> {
trimInPlace(s);
viewModel.setFamilyName(s.toString());
}));

this.finishButton.setOnClickListener(v -> {
this.finishButton.setIndeterminateProgressMode(true);
this.finishButton.setProgress(50);
handleUpload();
});

this.finishButton.setText(getArguments().getInt(NEXT_BUTTON_TEXT, R.string.CreateProfileActivity_next));
this.finishButton.setText(arguments.getInt(NEXT_BUTTON_TEXT, R.string.CreateProfileActivity_next));

this.usernameEditButton.setOnClickListener(v -> {
NavDirections action = EditProfileFragmentDirections.actionEditUsername();
Expand All @@ -238,8 +239,10 @@ private void initializeProfileName() {
updateFieldIfNeeded(givenName, profileName.getGivenName());
updateFieldIfNeeded(familyName, profileName.getFamilyName());

finishButton.setEnabled(!profileName.isEmpty());
finishButton.setAlpha(!profileName.isEmpty() ? 1f : 0.5f);
boolean validEntry = !profileName.isGivenNameEmpty();

finishButton.setEnabled(validEntry);
finishButton.setAlpha(validEntry ? 1f : 0.5f);

preview.setText(profileName.toString());
});
Expand All @@ -260,9 +263,11 @@ private void initializeUsername() {
viewModel.username().observe(this, this::onUsernameChanged);
}

private void updateFieldIfNeeded(@NonNull EditText field, @NonNull String value) {
if (!field.getText().toString().equals(value)) {
private static void updateFieldIfNeeded(@NonNull EditText field, @NonNull String value) {
String fieldTrimmed = field.getText().toString().trim();
String valueTrimmed = value.trim();

if (!fieldTrimmed.equals(valueTrimmed)) {
boolean setSelectionToEnd = field.getText().length() == 0;

field.setText(value);
Expand All @@ -273,24 +278,22 @@ private void updateFieldIfNeeded(@NonNull EditText field, @NonNull String value)
}
}

@SuppressLint("SetTextI18n")
private void onUsernameChanged(@NonNull Optional<String> username) {
if (username.isPresent()) {
this.username.setText("@" + username.get());
} else {
this.username.setText("");
}
this.username.setText(username.transform(s -> "@" + s).or(""));
}

private void startAvatarSelection() {
captureFile = AvatarSelection.startAvatarSelection(this, viewModel.hasAvatar(), true);
}

private void handleUpload() {

viewModel.submitProfile(uploadResult -> {
if (uploadResult == EditProfileRepository.UploadResult.SUCCESS) {
if (captureFile != null) captureFile.delete();
if (captureFile != null) {
if (!captureFile.delete()) {
Log.w(TAG, "Failed to delete capture file " + captureFile);
}
}
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) handleFinishedLollipop();
else handleFinishedLegacy();
} else {
Expand Down Expand Up @@ -345,6 +348,13 @@ public void onAnimationRepeat(Animator animation) {}
animation.start();
}

private static void trimInPlace(Editable s) {
int trimmedLength = ProfileName.trimToFit(s.toString()).length();
if (s.length() > trimmedLength) {
s.delete(trimmedLength, s.length());
}
}

public interface Controller {
void onProfileNameUploadCompleted();
}
Expand Down
6 changes: 1 addition & 5 deletions app/src/main/java/org/thoughtcrime/securesms/profiles/edit/EditProfileViewModel.java
View file
Expand Up @@ -20,7 +20,7 @@ class EditProfileViewModel extends ViewModel {
pair -> ProfileName.fromParts(pair.first(), pair.second()));
private final MutableLiveData<byte[]> internalAvatar = new MutableLiveData<>();
private final MutableLiveData<Optional<String>> internalUsername = new MutableLiveData<>();
private final EditProfileRepository repository;
private final EditProfileRepository repository;

private EditProfileViewModel(@NonNull EditProfileRepository repository) {
this.repository = repository;
Expand Down Expand Up @@ -70,10 +70,6 @@ public void submitProfile(Consumer<EditProfileRepository.UploadResult> uploadRes
repository.uploadProfile(profileName, internalAvatar.getValue(), uploadResultConsumer);
}

private ProfileName currentProfileName() {
return internalProfileName.getValue();
}

static class Factory implements ViewModelProvider.Factory {

private final EditProfileRepository repository;
Expand Down
4 changes: 2 additions & 2 deletions app/src/main/res/layout/profile_create_fragment.xml
View file
Expand Up @@ -120,9 +120,9 @@
android:layout_marginTop="13dp"
android:layout_marginEnd="16dp"
android:layout_weight="1"
android:autofillHints="given-name"
android:hint="@string/CreateProfileActivity_first_name_required"
android:inputType="textPersonName"
android:maxLength="@integer/profile_name_part_max_length"
android:singleLine="true" />

<EditText
Expand All @@ -135,9 +135,9 @@
android:layout_marginTop="13dp"
android:layout_marginEnd="16dp"
android:layout_weight="1"
android:autofillHints="family-name"
android:hint="@string/CreateProfileActivity_last_name_optional"
android:inputType="textPersonName"
android:maxLength="@integer/profile_name_part_max_length"
android:singleLine="true" />
</LinearLayout>

Expand Down
2 changes: 0 additions & 2 deletions app/src/main/res/values/integers.xml
View file
Expand Up @@ -4,6 +4,4 @@
<integer name="reaction_scrubber_reveal_duration">400</integer>
<integer name="reaction_scrubber_reveal_emoji_duration">380</integer>
<integer name="reaction_scrubber_emoji_reveal_duration_start_delay_factor">10</integer>

<integer name="profile_name_part_max_length">26</integer>
</resources>
19 changes: 17 additions & 2 deletions app/src/test/java/org/thoughtcrime/securesms/profiles/ProfileNameTest.java
View file
Expand Up @@ -10,7 +10,7 @@

public final class ProfileNameTest {

@Test
@Test
public void givenEmpty_thenIExpectSaneDefaults() {
// GIVEN
ProfileName profileName = ProfileName.EMPTY;
Expand All @@ -23,7 +23,7 @@ public void givenEmpty_thenIExpectSaneDefaults() {
}

@Test
public void givenNullProfileName_whenIFromDataString_thenIExpectSaneDefaults() {
public void givenNullProfileName_whenIFromSerialized_thenIExpectExactlyEmpty() {
// GIVEN
ProfileName profileName = ProfileName.fromSerialized(null);

Expand Down Expand Up @@ -128,6 +128,7 @@ public void givenProfileNameWithEmptyGivenName_whenIToDataString_thenIExpectAnEm

// THEN
assertEquals("Blank String should be returned (For back compat)", "", data);
assertEquals("Family", name.getFamilyName());
}

@Test
Expand Down Expand Up @@ -161,4 +162,18 @@ public void fromParts_with_long_name_parts() {
assertEquals("GivenSomeVeryLongNameSomeV", name.getGivenName());
assertEquals("FamilySomeVeryLongNameSome", name.getFamilyName());
}

@Test
public void givenProfileNameWithGivenNameAndFamilyNameWithSpaces_whenIToDataString_thenIExpectTrimmedProfileName() {
// GIVEN
ProfileName name = ProfileName.fromParts(" Given ", " Family");

// WHEN
String data = name.serialize();

// THEN
assertEquals(data, "Given\0Family");
assertEquals(name.getGivenName(), "Given");
assertEquals(name.getFamilyName(), "Family");
}
}

0 comments on commit e7f568e

Please sign in to comment.