NewSearchFragment contact photos now properly open quick contact cards.
There was an issue where businesses' and remote contacts' contact photos
wouldn't open contact cards correctly. The issue was rooted in the incorrect
contact uri being assigned to the quick contact badge.
from the bugbash:
16. Tap on business icon from search results says “ no application found” instead of opening the business info
17. Same as #16 but with contact from Directory Google.com - “The contact doesn’t exist” when tapping contact icon
Bug: 64902476
Test: existing
PiperOrigin-RevId: 167111016
Change-Id: I4b6f7ca812d2fc4dc220951e8c05db2c8b8d6114
diff --git a/java/com/android/dialer/searchfragment/common/SearchCursor.java b/java/com/android/dialer/searchfragment/common/SearchCursor.java
index 368ee09..7ad19aa 100644
--- a/java/com/android/dialer/searchfragment/common/SearchCursor.java
+++ b/java/com/android/dialer/searchfragment/common/SearchCursor.java
@@ -35,4 +35,11 @@
* @return true if the data set has changed.
*/
boolean updateQuery(@NonNull String query);
+
+ /**
+ * Returns an ID unique to the directory this cursor reads from. Generally this value will be
+ * related to {@link android.provider.ContactsContract.Directory} but could differ depending on
+ * the implementation.
+ */
+ long getDirectoryId();
}
diff --git a/java/com/android/dialer/searchfragment/cp2/SearchContactsCursor.java b/java/com/android/dialer/searchfragment/cp2/SearchContactsCursor.java
index 18c9ecd..508ca7f 100644
--- a/java/com/android/dialer/searchfragment/cp2/SearchContactsCursor.java
+++ b/java/com/android/dialer/searchfragment/cp2/SearchContactsCursor.java
@@ -20,6 +20,7 @@
import android.database.Cursor;
import android.database.MatrixCursor;
import android.database.MergeCursor;
+import android.provider.ContactsContract.Directory;
import android.support.annotation.Nullable;
import com.android.dialer.searchfragment.common.SearchCursor;
@@ -32,7 +33,7 @@
private final ContactFilterCursor contactFilterCursor;
- public static SearchContactsCursor newInstnace(
+ static SearchContactsCursor newInstance(
Context context, ContactFilterCursor contactFilterCursor) {
MatrixCursor headerCursor = new MatrixCursor(HEADER_PROJECTION);
headerCursor.addRow(new String[] {context.getString(R.string.all_contacts)});
@@ -56,6 +57,11 @@
}
@Override
+ public long getDirectoryId() {
+ return Directory.DEFAULT;
+ }
+
+ @Override
public int getCount() {
// If we don't have any contents, we don't want to show the header
int count = contactFilterCursor.getCount();
diff --git a/java/com/android/dialer/searchfragment/cp2/SearchContactsCursorLoader.java b/java/com/android/dialer/searchfragment/cp2/SearchContactsCursorLoader.java
index 84fd64a..b7fc9b5 100644
--- a/java/com/android/dialer/searchfragment/cp2/SearchContactsCursorLoader.java
+++ b/java/com/android/dialer/searchfragment/cp2/SearchContactsCursorLoader.java
@@ -47,6 +47,6 @@
// Filtering logic
ContactFilterCursor contactFilterCursor = new ContactFilterCursor(cursor, query);
// Header logic
- return SearchContactsCursor.newInstnace(getContext(), contactFilterCursor);
+ return SearchContactsCursor.newInstance(getContext(), contactFilterCursor);
}
}
diff --git a/java/com/android/dialer/searchfragment/list/NewSearchFragment.java b/java/com/android/dialer/searchfragment/list/NewSearchFragment.java
index 7fee969..910e454 100644
--- a/java/com/android/dialer/searchfragment/list/NewSearchFragment.java
+++ b/java/com/android/dialer/searchfragment/list/NewSearchFragment.java
@@ -120,7 +120,6 @@
private void initLoaders() {
getLoaderManager().initLoader(CONTACTS_LOADER_ID, null, this);
- loadNearbyPlacesCursor();
loadRemoteDirectoriesCursor();
}
@@ -129,7 +128,14 @@
if (id == CONTACTS_LOADER_ID) {
return new SearchContactsCursorLoader(getContext(), query);
} else if (id == NEARBY_PLACES_LOADER_ID) {
- return new NearbyPlacesCursorLoader(getContext(), query);
+ // Directories represent contact data sources on the device, but since nearby places aren't
+ // stored on the device, they don't have a directory ID. We pass the list of all existing IDs
+ // so that we can find one that doesn't collide.
+ List<Integer> directoryIds = new ArrayList<>();
+ for (Directory directory : directories) {
+ directoryIds.add(directory.getId());
+ }
+ return new NearbyPlacesCursorLoader(getContext(), query, directoryIds);
} else if (id == REMOTE_DIRECTORIES_LOADER_ID) {
return new RemoteDirectoriesCursorLoader(getContext());
} else if (id == REMOTE_CONTACTS_LOADER_ID) {
@@ -162,6 +168,7 @@
while (cursor.moveToNext()) {
directories.add(RemoteDirectoriesCursorLoader.readDirectory(cursor));
}
+ loadNearbyPlacesCursor();
loadRemoteContactsCursors();
} else {
@@ -212,18 +219,6 @@
ThreadUtil.getUiThreadHandler().removeCallbacks(capabilitiesUpdatedRunnable);
}
- private void loadNearbyPlacesCursor() {
- // Cancel existing load if one exists.
- ThreadUtil.getUiThreadHandler().removeCallbacks(loadNearbyPlacesRunnable);
-
- // If nearby places is not enabled, do not try to load them.
- if (!PhoneDirectoryExtenderAccessor.get(getContext()).isEnabled(getContext())) {
- return;
- }
- ThreadUtil.getUiThreadHandler()
- .postDelayed(loadNearbyPlacesRunnable, NETWORK_SEARCH_DELAY_MILLIS);
- }
-
@Override
public void onRequestPermissionsResult(
int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) {
@@ -250,12 +245,14 @@
}
}
+ // Loads remote directories.
private void loadRemoteDirectoriesCursor() {
if (!remoteDirectoriesDisabledForTesting) {
getLoaderManager().initLoader(REMOTE_DIRECTORIES_LOADER_ID, null, this);
}
}
+ // Should not be called before remote directories have finished loading.
private void loadRemoteContactsCursors() {
if (remoteDirectoriesDisabledForTesting) {
return;
@@ -267,6 +264,19 @@
.postDelayed(loadRemoteContactsRunnable, NETWORK_SEARCH_DELAY_MILLIS);
}
+ // Should not be called before remote directories (not contacts) have finished loading.
+ private void loadNearbyPlacesCursor() {
+ // Cancel existing load if one exists.
+ ThreadUtil.getUiThreadHandler().removeCallbacks(loadNearbyPlacesRunnable);
+
+ // If nearby places is not enabled, do not try to load them.
+ if (!PhoneDirectoryExtenderAccessor.get(getContext()).isEnabled(getContext())) {
+ return;
+ }
+ ThreadUtil.getUiThreadHandler()
+ .postDelayed(loadNearbyPlacesRunnable, NETWORK_SEARCH_DELAY_MILLIS);
+ }
+
@Override
public void onResume() {
super.onResume();
diff --git a/java/com/android/dialer/searchfragment/nearbyplaces/NearbyPlaceViewHolder.java b/java/com/android/dialer/searchfragment/nearbyplaces/NearbyPlaceViewHolder.java
index 575582e..fa07826 100644
--- a/java/com/android/dialer/searchfragment/nearbyplaces/NearbyPlaceViewHolder.java
+++ b/java/com/android/dialer/searchfragment/nearbyplaces/NearbyPlaceViewHolder.java
@@ -17,13 +17,14 @@
package com.android.dialer.searchfragment.nearbyplaces;
import android.content.Context;
-import android.database.Cursor;
import android.net.Uri;
import android.provider.ContactsContract;
+import android.provider.ContactsContract.Contacts;
import android.support.v7.widget.RecyclerView;
import android.view.View;
import android.widget.QuickContactBadge;
import android.widget.TextView;
+import com.android.contacts.common.util.Constants;
import com.android.dialer.callintent.CallInitiationType;
import com.android.dialer.callintent.CallIntentBuilder;
import com.android.dialer.contactphoto.ContactPhotoManager;
@@ -31,6 +32,7 @@
import com.android.dialer.searchfragment.common.Projections;
import com.android.dialer.searchfragment.common.QueryBoldingUtil;
import com.android.dialer.searchfragment.common.R;
+import com.android.dialer.searchfragment.common.SearchCursor;
import com.android.dialer.telecom.TelecomUtil;
/** ViewHolder for a nearby place row. */
@@ -57,14 +59,13 @@
* Binds the ViewHolder with a cursor from {@link NearbyPlacesCursorLoader} with the data found at
* the cursors set position.
*/
- public void bind(Cursor cursor, String query) {
+ public void bind(SearchCursor cursor, String query) {
number = cursor.getString(Projections.PHONE_NUMBER);
String name = cursor.getString(Projections.PHONE_DISPLAY_NAME);
String address = cursor.getString(Projections.PHONE_LABEL);
placeName.setText(QueryBoldingUtil.getNameWithQueryBolded(query, name));
placeAddress.setText(QueryBoldingUtil.getNameWithQueryBolded(query, address));
-
String photoUri = cursor.getString(Projections.PHONE_PHOTO_URI);
ContactPhotoManager.getInstance(context)
.loadDialerThumbnailOrPhoto(
@@ -73,13 +74,21 @@
cursor.getLong(Projections.PHONE_PHOTO_ID),
photoUri == null ? null : Uri.parse(photoUri),
name,
- LetterTileDrawable.TYPE_DEFAULT);
+ LetterTileDrawable.TYPE_BUSINESS);
}
- private static Uri getContactUri(Cursor cursor) {
- long contactId = cursor.getLong(Projections.PHONE_ID);
- String lookupKey = cursor.getString(Projections.PHONE_LOOKUP_KEY);
- return ContactsContract.Contacts.getLookupUri(contactId, lookupKey);
+ private static Uri getContactUri(SearchCursor cursor) {
+ // Since the lookup key for Nearby Places is actually a JSON representation of the information,
+ // we need to pass it in as an encoded fragment in our contact uri.
+ // It includes information like display name, photo uri, phone number, ect.
+ String businessInfoJson = cursor.getString(Projections.PHONE_LOOKUP_KEY);
+ return Contacts.CONTENT_LOOKUP_URI
+ .buildUpon()
+ .appendPath(Constants.LOOKUP_URI_ENCODED)
+ .appendQueryParameter(
+ ContactsContract.DIRECTORY_PARAM_KEY, String.valueOf(cursor.getDirectoryId()))
+ .encodedFragment(businessInfoJson)
+ .build();
}
@Override
diff --git a/java/com/android/dialer/searchfragment/nearbyplaces/NearbyPlacesCursor.java b/java/com/android/dialer/searchfragment/nearbyplaces/NearbyPlacesCursor.java
index a4142a4..3be59b6 100644
--- a/java/com/android/dialer/searchfragment/nearbyplaces/NearbyPlacesCursor.java
+++ b/java/com/android/dialer/searchfragment/nearbyplaces/NearbyPlacesCursor.java
@@ -27,16 +27,23 @@
final class NearbyPlacesCursor extends MergeCursor implements SearchCursor {
private final Cursor nearbyPlacesCursor;
+ private final long directoryId;
- public static NearbyPlacesCursor newInstnace(Context context, Cursor nearbyPlacesCursor) {
+ /**
+ * @param directoryId unique directory id that doesn't collide with other remote/local
+ * directories. directoryIds are needed to load the correct quick contact card.
+ */
+ static NearbyPlacesCursor newInstance(
+ Context context, Cursor nearbyPlacesCursor, long directoryId) {
MatrixCursor headerCursor = new MatrixCursor(HEADER_PROJECTION);
headerCursor.addRow(new String[] {context.getString(R.string.nearby_places)});
- return new NearbyPlacesCursor(new Cursor[] {headerCursor, nearbyPlacesCursor});
+ return new NearbyPlacesCursor(new Cursor[] {headerCursor, nearbyPlacesCursor}, directoryId);
}
- private NearbyPlacesCursor(Cursor[] cursors) {
+ private NearbyPlacesCursor(Cursor[] cursors, long directoryId) {
super(cursors);
nearbyPlacesCursor = cursors[1];
+ this.directoryId = directoryId;
}
@Override
@@ -61,4 +68,9 @@
int count = nearbyPlacesCursor.getCount();
return count == 0 ? 0 : count + 1;
}
+
+ @Override
+ public long getDirectoryId() {
+ return directoryId;
+ }
}
diff --git a/java/com/android/dialer/searchfragment/nearbyplaces/NearbyPlacesCursorLoader.java b/java/com/android/dialer/searchfragment/nearbyplaces/NearbyPlacesCursorLoader.java
index 6807a6e..c8bb36a 100644
--- a/java/com/android/dialer/searchfragment/nearbyplaces/NearbyPlacesCursorLoader.java
+++ b/java/com/android/dialer/searchfragment/nearbyplaces/NearbyPlacesCursorLoader.java
@@ -21,21 +21,37 @@
import android.database.Cursor;
import android.net.Uri;
import android.provider.ContactsContract;
+import android.support.annotation.NonNull;
import com.android.contacts.common.extensions.PhoneDirectoryExtenderAccessor;
+import com.android.dialer.common.LogUtil;
import com.android.dialer.searchfragment.common.Projections;
+import java.util.List;
/** Cursor loader for nearby places search results. */
public final class NearbyPlacesCursorLoader extends CursorLoader {
private static final String MAX_RESULTS = "3";
+ private static final long INVALID_DIRECTORY_ID = Long.MAX_VALUE;
+ private final long directoryId;
- public NearbyPlacesCursorLoader(Context context, String query) {
+ /**
+ * @param directoryIds List of directoryIds associated with all directories on device. Required in
+ * order to find a directory ID for the nearby places cursor that doesn't collide with
+ * existing directories.
+ */
+ public NearbyPlacesCursorLoader(
+ Context context, String query, @NonNull List<Integer> directoryIds) {
super(context, getContentUri(context, query), Projections.PHONE_PROJECTION, null, null, null);
+ this.directoryId = getDirectoryId(directoryIds);
}
@Override
public Cursor loadInBackground() {
- return NearbyPlacesCursor.newInstnace(getContext(), super.loadInBackground());
+ if (directoryId == INVALID_DIRECTORY_ID) {
+ LogUtil.i("NearbyPlacesCursorLoader.loadInBackground", "directory id not set.");
+ return null;
+ }
+ return NearbyPlacesCursor.newInstance(getContext(), super.loadInBackground(), directoryId);
}
private static Uri getContentUri(Context context, String query) {
@@ -46,4 +62,22 @@
.appendQueryParameter(ContactsContract.LIMIT_PARAM_KEY, MAX_RESULTS)
.build();
}
+
+ private static long getDirectoryId(List<Integer> directoryIds) {
+ if (directoryIds.isEmpty()) {
+ return INVALID_DIRECTORY_ID;
+ }
+
+ // The Directory.LOCAL_INVISIBLE might not be a directory we use, but we can't reuse it's
+ // "special" ID.
+ long maxId = ContactsContract.Directory.LOCAL_INVISIBLE;
+ for (int i = 0, n = directoryIds.size(); i < n; i++) {
+ long id = directoryIds.get(i);
+ if (id > maxId) {
+ maxId = id;
+ }
+ }
+ // Add one so that the nearby places ID doesn't collide with extended directory IDs.
+ return maxId + 1;
+ }
}
diff --git a/java/com/android/dialer/searchfragment/remote/RemoteContactViewHolder.java b/java/com/android/dialer/searchfragment/remote/RemoteContactViewHolder.java
index 5fb12d3..df3eacc 100644
--- a/java/com/android/dialer/searchfragment/remote/RemoteContactViewHolder.java
+++ b/java/com/android/dialer/searchfragment/remote/RemoteContactViewHolder.java
@@ -22,6 +22,7 @@
import android.net.Uri;
import android.provider.ContactsContract;
import android.provider.ContactsContract.CommonDataKinds.Phone;
+import android.provider.ContactsContract.Contacts;
import android.support.v7.widget.RecyclerView;
import android.text.TextUtils;
import android.view.View;
@@ -119,10 +120,14 @@
return (String) Phone.getTypeLabel(resources, numberType, numberLabel);
}
- private static Uri getContactUri(Cursor cursor) {
+ private static Uri getContactUri(SearchCursor cursor) {
long contactId = cursor.getLong(Projections.PHONE_ID);
String lookupKey = cursor.getString(Projections.PHONE_LOOKUP_KEY);
- return ContactsContract.Contacts.getLookupUri(contactId, lookupKey);
+ return Contacts.getLookupUri(contactId, lookupKey)
+ .buildUpon()
+ .appendQueryParameter(
+ ContactsContract.DIRECTORY_PARAM_KEY, String.valueOf(cursor.getDirectoryId()))
+ .build();
}
@Override
diff --git a/java/com/android/dialer/searchfragment/remote/RemoteContactsCursor.java b/java/com/android/dialer/searchfragment/remote/RemoteContactsCursor.java
index d7c4f38..e6f3c26 100644
--- a/java/com/android/dialer/searchfragment/remote/RemoteContactsCursor.java
+++ b/java/com/android/dialer/searchfragment/remote/RemoteContactsCursor.java
@@ -26,6 +26,7 @@
import com.android.dialer.searchfragment.common.SearchCursor;
import com.android.dialer.searchfragment.remote.RemoteDirectoriesCursorLoader.Directory;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.List;
/**
@@ -41,6 +42,16 @@
public final class RemoteContactsCursor extends MergeCursor implements SearchCursor {
/**
+ * {@link SearchCursor#HEADER_PROJECTION} with {@link #COLUMN_DIRECTORY_ID} appended on the end.
+ *
+ * <p>This is needed to get the directoryId associated with each contact. directoryIds are needed
+ * to load the correct quick contact card.
+ */
+ private static final String[] PROJECTION = buildProjection();
+
+ private static final String COLUMN_DIRECTORY_ID = "directory_id";
+
+ /**
* Returns a single cursor with headers inserted between each non-empty cursor. If all cursors are
* empty, null or closed, this method returns null.
*/
@@ -78,18 +89,24 @@
continue;
}
- cursorList.add(createHeaderCursor(context, directory.getDisplayName()));
+ cursorList.add(createHeaderCursor(context, directory.getDisplayName(), directory.getId()));
cursorList.add(cursor);
}
return cursorList.toArray(new Cursor[cursorList.size()]);
}
- private static MatrixCursor createHeaderCursor(Context context, String name) {
- MatrixCursor headerCursor = new MatrixCursor(HEADER_PROJECTION, 1);
- headerCursor.addRow(new String[] {context.getString(R.string.directory, name)});
+ private static MatrixCursor createHeaderCursor(Context context, String name, int id) {
+ MatrixCursor headerCursor = new MatrixCursor(PROJECTION, 1);
+ headerCursor.addRow(new Object[] {context.getString(R.string.directory, name), id});
return headerCursor;
}
+ private static String[] buildProjection() {
+ String[] projection = Arrays.copyOf(HEADER_PROJECTION, HEADER_PROJECTION.length + 1);
+ projection[projection.length - 1] = COLUMN_DIRECTORY_ID;
+ return projection;
+ }
+
/** Returns true if the current position is a header row. */
@Override
public boolean isHeader() {
@@ -97,6 +114,21 @@
}
@Override
+ public long getDirectoryId() {
+ int position = getPosition();
+ // proceed backwards until we reach the header row, which contains the directory ID.
+ while (moveToPrevious()) {
+ int id = getInt(getColumnIndex(COLUMN_DIRECTORY_ID));
+ if (id != -1) {
+ // return the cursor to it's original position/state
+ moveToPosition(position);
+ return id;
+ }
+ }
+ throw Assert.createIllegalStateFailException("No directory id for contact at: " + position);
+ }
+
+ @Override
public boolean updateQuery(@Nullable String query) {
// When the query changes, a new network request is made for nearby places. Meaning this cursor
// will be closed and another created, so return false.
diff --git a/java/com/android/dialer/searchfragment/remote/RemoteDirectoriesCursorLoader.java b/java/com/android/dialer/searchfragment/remote/RemoteDirectoriesCursorLoader.java
index 327a62c..de71025 100644
--- a/java/com/android/dialer/searchfragment/remote/RemoteDirectoriesCursorLoader.java
+++ b/java/com/android/dialer/searchfragment/remote/RemoteDirectoriesCursorLoader.java
@@ -67,7 +67,7 @@
return new AutoValue_RemoteDirectoriesCursorLoader_Directory(id, displayName, supportsPhotos);
}
- abstract int getId();
+ public abstract int getId();
/** Returns a user facing display name of the directory. Null if none exists. */
abstract @Nullable String getDisplayName();
diff --git a/java/com/android/dialer/searchfragment/testing/TestSearchCursor.java b/java/com/android/dialer/searchfragment/testing/TestSearchCursor.java
index 9a0b957..7e6299e 100644
--- a/java/com/android/dialer/searchfragment/testing/TestSearchCursor.java
+++ b/java/com/android/dialer/searchfragment/testing/TestSearchCursor.java
@@ -44,4 +44,9 @@
public boolean updateQuery(@Nullable String query) {
return false;
}
+
+ @Override
+ public long getDirectoryId() {
+ return 0;
+ }
}