Bug 1366678 - Highlight substrings in search suggestion/history if they match search term. r=walkingice
authorjwu <topwu.tw@gmail.com>
Wed, 19 Jul 2017 15:08:48 +0800
changeset 418375 967111e5be01c016ba1332f7985729efc4d07fa2
parent 418374 b11f89deba4e240ea0cbfafac6d2eb3488e2699a
child 418376 3351fac5174b5036f7d468766d1575e42c8fcbf4
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerswalkingice
bugs1366678
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1366678 - Highlight substrings in search suggestion/history if they match search term. r=walkingice MozReview-Commit-ID: KwpgBaaSYn1
mobile/android/app/src/photon/java/org/mozilla/gecko/home/SearchEngineRow.java
mobile/android/app/src/photon/res/layout/suggestion_item.xml
mobile/android/app/src/photon/res/values-v16/styles.xml
mobile/android/app/src/photon/res/values/styles.xml
mobile/android/base/java/org/mozilla/gecko/home/BrowserSearch.java
mobile/android/base/java/org/mozilla/gecko/home/TwoLinePageRow.java
mobile/android/base/java/org/mozilla/gecko/widget/FadedSingleColorTextView.java
--- a/mobile/android/app/src/photon/java/org/mozilla/gecko/home/SearchEngineRow.java
+++ b/mobile/android/app/src/photon/java/org/mozilla/gecko/home/SearchEngineRow.java
@@ -210,24 +210,20 @@ class SearchEngineRow extends ThemedRela
         }
 
         final TextView suggestionText = (TextView) v.findViewById(R.id.suggestion_text);
         final String searchTerm = getSuggestionTextFromView(mUserEnteredView);
         final int searchTermLength = searchTerm.length();
         refreshOccurrencesWith(searchTerm, suggestion);
         if (mOccurrences.size() > 0) {
             final SpannableStringBuilder sb = new SpannableStringBuilder(suggestion);
-            int nextStartSpanIndex = 0;
-            // Done to make sure that the stretch of text after the last occurrence, till the end of the suggestion, is made bold
-            mOccurrences.add(suggestion.length());
             for (int occurrence : mOccurrences) {
                 // Even though they're the same style, SpannableStringBuilder will interpret there as being only one Span present if we re-use a StyleSpan
-                StyleSpan boldSpan = new StyleSpan(Typeface.BOLD);
-                sb.setSpan(boldSpan, nextStartSpanIndex, occurrence, Spannable.SPAN_INCLUSIVE_INCLUSIVE);
-                nextStartSpanIndex = occurrence + searchTermLength;
+                final StyleSpan boldSpan = new StyleSpan(Typeface.BOLD);
+                sb.setSpan(boldSpan, occurrence, occurrence + searchTermLength, Spannable.SPAN_INCLUSIVE_INCLUSIVE);
             }
             mOccurrences.clear();
             suggestionText.setText(sb);
         } else {
             suggestionText.setText(suggestion);
         }
 
         setDescriptionOnSuggestion(suggestionText, suggestion);
@@ -240,17 +236,20 @@ class SearchEngineRow extends ThemedRela
         String searchTerm = getSuggestionTextFromView(mUserEnteredView);
         if (mSearchListener != null) {
             Telemetry.sendUIEvent(TelemetryContract.Event.LOAD_URL, TelemetryContract.Method.SUGGESTION, "user");
             mSearchListener.onSearch(mSearchEngine, searchTerm, TelemetryContract.Method.SUGGESTION);
         }
     }
 
     public void setSearchTerm(String searchTerm) {
-        mUserEnteredTextView.setText(searchTerm);
+        final SpannableStringBuilder sb = new SpannableStringBuilder(searchTerm);
+        final StyleSpan boldSpan = new StyleSpan(Typeface.BOLD);
+        sb.setSpan(boldSpan, 0, searchTerm.length(), Spannable.SPAN_INCLUSIVE_INCLUSIVE);
+        mUserEnteredTextView.setText(sb);
 
         // mSearchEngine is not set in the first call to this method; the content description
         // is instead initially set in updateSuggestions().
         if (mSearchEngine != null) {
             setDescriptionOnSuggestion(mUserEnteredTextView, searchTerm);
         }
     }
 
--- a/mobile/android/app/src/photon/res/layout/suggestion_item.xml
+++ b/mobile/android/app/src/photon/res/layout/suggestion_item.xml
@@ -25,18 +25,18 @@
         android:layout_marginEnd="3dip"
         android:layout_marginRight="3dip"
         android:src="@drawable/icon_most_recent_empty"
         android:visibility="gone"
         tools:visibility="visible"/>
 
     <org.mozilla.gecko.widget.themed.ThemedTextView
         android:id="@+id/suggestion_text"
+        style="@style/TextAppearance.SearchSuggestion"
         android:layout_width="wrap_content"
         android:layout_height="wrap_content"
         android:layout_gravity="center_vertical"
         android:gravity="center_vertical"
         android:lineSpacingMultiplier="1.1"
         android:textColor="@color/search_suggestion_text"
-        android:textSize="14sp"
         tools:text="suggestion text"/>
 
 </org.mozilla.gecko.home.SuggestionItem>
--- a/mobile/android/app/src/photon/res/values-v16/styles.xml
+++ b/mobile/android/app/src/photon/res/values-v16/styles.xml
@@ -21,9 +21,12 @@
     <style name="TextAppearance.FirstrunTextRegular">
         <item name="android:fontFamily">sans-serif</item>
     </style>
 
     <style name="TextAppearance.UrlBar.Title" parent="TextAppearance.Small">
         <item name="android:textSize">16sp</item>
     </style>
 
+    <style name="TextAppearance.SearchSuggestion" parent="TextAppearance.Small">
+        <item name="android:fontFamily">sans-serif-light</item>
+    </style>
 </resources>
--- a/mobile/android/app/src/photon/res/values/styles.xml
+++ b/mobile/android/app/src/photon/res/values/styles.xml
@@ -427,16 +427,18 @@
     <style name="TextAppearance.DoorHanger.Small">
         <item name="android:textSize">14sp</item>
     </style>
 
     <style name="TextAppearance.UrlBar.Title" parent="TextAppearance.Small">
         <item name="android:textSize">16sp</item>
     </style>
 
+    <style name="TextAppearance.SearchSuggestion" parent="TextAppearance.Small"/>
+
     <!-- BrowserToolbar -->
     <style name="BrowserToolbar">
         <item name="android:layout_width">match_parent</item>
         <item name="android:layout_height">@dimen/browser_toolbar_height</item>
         <item name="android:orientation">horizontal</item>
     </style>
 
     <!-- URL bar -->
--- a/mobile/android/base/java/org/mozilla/gecko/home/BrowserSearch.java
+++ b/mobile/android/base/java/org/mozilla/gecko/home/BrowserSearch.java
@@ -28,26 +28,33 @@ import org.mozilla.gecko.Telemetry;
 import org.mozilla.gecko.TelemetryContract;
 import org.mozilla.gecko.annotation.RobocopTarget;
 import org.mozilla.gecko.db.BrowserContract;
 import org.mozilla.gecko.db.BrowserContract.History;
 import org.mozilla.gecko.db.BrowserContract.URLColumns;
 import org.mozilla.gecko.home.HomePager.OnUrlOpenListener;
 import org.mozilla.gecko.home.SearchLoader.SearchCursorLoader;
 import org.mozilla.gecko.preferences.GeckoPreferences;
+import org.mozilla.gecko.skin.SkinConfig;
 import org.mozilla.gecko.toolbar.AutocompleteHandler;
 import org.mozilla.gecko.util.BundleEventListener;
 import org.mozilla.gecko.util.EventCallback;
 import org.mozilla.gecko.util.GeckoBundle;
 import org.mozilla.gecko.util.StringUtils;
 import org.mozilla.gecko.util.ThreadUtils;
 
 import android.app.Activity;
 import android.content.ContentResolver;
 import android.content.Context;
+import android.graphics.Typeface;
+import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
+import android.text.Spannable;
+import android.text.SpannableStringBuilder;
+import android.text.style.StyleSpan;
 import android.view.ContextMenu.ContextMenuInfo;
 import android.database.Cursor;
 import android.net.Uri;
 import android.os.Bundle;
 import android.support.v4.app.LoaderManager.LoaderCallbacks;
 import android.support.v4.content.AsyncTaskLoader;
 import android.support.v4.content.Loader;
 import android.text.TextUtils;
@@ -1184,22 +1191,57 @@ public class BrowserSearch extends HomeF
                 row.updateSuggestions(mSuggestionsEnabled, engine, mSearchHistorySuggestions);
                 row.setPrivateMode(isPrivate);
             } else {
                 // Account for the search engines
                 position -= getPrimaryEngineCount();
 
                 final Cursor c = getCursor(position);
                 final TwoLinePageRow row = (TwoLinePageRow) view;
+
+                if (SkinConfig.isPhoton()) {
+                    // Highlight all substrings in title field if they matches the search term.
+                    row.setTitleFormatter(mTwoLinePageRowTitleFormatter);
+                }
                 row.updateFromCursor(c);
                 row.setPrivateMode(isPrivate);
             }
         }
     }
 
+    private TwoLinePageRow.TitleFormatter mTwoLinePageRowTitleFormatter = new TwoLinePageRow.TitleFormatter() {
+        @Override
+        public CharSequence format(@NonNull CharSequence title) {
+            // Don't try to search for an empty string - String.indexOf will return 0, which would result
+            // in us iterating with lastIndexOfMatch = 0, which eventually results in an OOM.
+            if (TextUtils.isEmpty(mSearchTerm)) {
+                return title;
+            }
+
+            // Find matching substrings in title field in TwoLinePageRow, ignoring cases.
+            final String titleInLowerCase = title.toString().toLowerCase();
+            final String pattern = mSearchTerm.toLowerCase();
+            final int patternLength = pattern.length();
+
+            final SpannableStringBuilder sb = new SpannableStringBuilder(title);
+
+            int indexOfMatch = 0;
+            int lastIndexOfMatch = 0;
+            while (indexOfMatch != -1) {
+                indexOfMatch = titleInLowerCase.indexOf(pattern, lastIndexOfMatch);
+                lastIndexOfMatch = indexOfMatch + patternLength;
+                if (indexOfMatch != -1) {
+                    final StyleSpan boldSpan = new StyleSpan(Typeface.BOLD);
+                    sb.setSpan(boldSpan, indexOfMatch, lastIndexOfMatch, Spannable.SPAN_INCLUSIVE_INCLUSIVE);
+                }
+            }
+            return sb;
+        }
+    };
+
     private class CursorLoaderCallbacks implements LoaderCallbacks<Cursor> {
         @Override
         public Loader<Cursor> onCreateLoader(int id, Bundle args) {
             return SearchLoader.createInstance(getActivity(), args);
         }
 
         @Override
         public void onLoadFinished(Loader<Cursor> loader, Cursor c) {
--- a/mobile/android/base/java/org/mozilla/gecko/home/TwoLinePageRow.java
+++ b/mobile/android/base/java/org/mozilla/gecko/home/TwoLinePageRow.java
@@ -4,18 +4,21 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.home;
 
 import java.util.concurrent.Future;
 
 import android.content.Context;
 import android.database.Cursor;
+import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
 import android.support.v4.view.ViewCompat;
 import android.support.v4.widget.TextViewCompat;
+import android.text.Spannable;
 import android.text.TextUtils;
 import android.util.AttributeSet;
 import android.view.Gravity;
 import android.view.LayoutInflater;
 import android.view.View;
 import android.widget.ImageView;
 
 import org.mozilla.gecko.R;
@@ -50,16 +53,18 @@ public class TwoLinePageRow extends Them
 
     private boolean mShowIcons;
 
     // The URL for the page corresponding to this view.
     private String mPageUrl;
 
     private boolean mHasReaderCacheItem;
 
+    private TitleFormatter mTitleFormatter;
+
     public TwoLinePageRow(Context context) {
         this(context, null);
     }
 
     public TwoLinePageRow(Context context, AttributeSet attrs) {
         super(context, attrs);
 
         setGravity(Gravity.CENTER_VERTICAL);
@@ -136,17 +141,17 @@ public class TwoLinePageRow extends Them
             case LOCATION_CHANGE:
                 updateDisplayedUrl();
                 break;
             default:
                 break;
         }
     }
 
-    private void setTitle(String text) {
+    private void setTitle(CharSequence text) {
         mTitle.setText(text);
     }
 
     protected void setUrl(String text) {
         mUrl.setText(text);
     }
 
     protected void setUrl(int stringId) {
@@ -244,17 +249,22 @@ public class TwoLinePageRow extends Them
 
             updateStatusIcon(isBookmark, hasReaderCacheItem);
         } else {
             updateStatusIcon(false, false);
         }
 
         // Use the URL instead of an empty title for consistency with the normal URL
         // bar view - this is the equivalent of getDisplayTitle() in Tab.java
-        setTitle(TextUtils.isEmpty(title) ? url : title);
+        final String titleToShow = TextUtils.isEmpty(title) ? url : title;
+        if (mTitleFormatter != null) {
+            setTitle(mTitleFormatter.format(titleToShow));
+        } else {
+            setTitle(titleToShow);
+        }
 
         // No point updating the below things if URL has not changed. Prevents evil Favicon flicker.
         if (url.equals(mPageUrl)) {
             return;
         }
 
         // Blank the Favicon, so we don't show the wrong Favicon if we scroll and miss DB.
         mFavicon.clearImage();
@@ -324,9 +334,18 @@ public class TwoLinePageRow extends Them
             bookmarkId = 0;
         }
 
         SavedReaderViewHelper rch = SavedReaderViewHelper.getSavedReaderViewHelper(getContext());
         final boolean hasReaderCacheItem = rch.isURLCached(url);
 
         update(title, url, bookmarkId, hasReaderCacheItem);
     }
+
+    public void setTitleFormatter(TitleFormatter formatter) {
+        mTitleFormatter = formatter;
+    }
+
+    // Use this interface to decorate content in title view.
+    interface TitleFormatter {
+        CharSequence format(@NonNull CharSequence title);
+    }
 }
--- a/mobile/android/base/java/org/mozilla/gecko/widget/FadedSingleColorTextView.java
+++ b/mobile/android/base/java/org/mozilla/gecko/widget/FadedSingleColorTextView.java
@@ -49,17 +49,18 @@ public class FadedSingleColorTextView ex
         getPaint().setShader(needsEllipsis ? mTextGradient : null);
     }
 
     @Override
     public void setText(CharSequence text, BufferType type) {
         super.setText(text, type);
         final boolean previousTextDirectionRtl = mIsTextDirectionRtl;
         if (!TextUtils.isEmpty(text)) {
-            mIsTextDirectionRtl = BidiFormatter.getInstance().isRtl((String) text);
+            // The text is an instance of CharSequence, not String. It cannot cast to String directly, use toString() instead.
+            mIsTextDirectionRtl = BidiFormatter.getInstance().isRtl(text.toString());
         }
         if (mIsTextDirectionRtl != previousTextDirectionRtl) {
             mTextGradient = null;
         }
         ViewUtil.setTextDirectionRtlCompat(this, mIsTextDirectionRtl);
     }
 
     @Override