Bug 1435060 - Allow keyword bookmark as search query; r?JanH draft
authorSnowrries <allentung5@gmail.com>
Wed, 14 Feb 2018 03:07:13 -0500
changeset 754802 d224778c614cdb4315b59a497a832b9b178867ee
parent 754572 e43f2f6ea111c2d059d95fa9a71516b869a69698
push id99003
push userbmo:allentung5@gmail.com
push dateWed, 14 Feb 2018 10:23:53 +0000
reviewersJanH
bugs1435060
milestone60.0a1
Bug 1435060 - Allow keyword bookmark as search query; r?JanH Function queryExists added to StringUtils to check if a given URL allows for arguments. Tests for said function added to TestStringUtils. Function loadUrlOrKeywordSearch in BrowserApp.java updated to also pass the entire entered string along regardless of whether or not the first words are keyword bookmarks iff there is additional text after the keyword bookmark, and the keyword bookmark does not accept any input arguments MozReview-Commit-ID: 2eyi5Botd3F
mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/StringUtils.java
mobile/android/geckoview/src/test/java/org/mozilla/gecko/util/TestStringUtils.java
--- a/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
@@ -2737,19 +2737,21 @@ public class BrowserApp extends GeckoApp
                     keywordSearch = "";
                 } else {
                     keyword = url.substring(0, index);
                     keywordSearch = url.substring(index + 1);
                 }
 
                 final String keywordUrl = db.getUrlForKeyword(getContentResolver(), keyword);
 
-                // If there isn't a bookmark keyword, load the url. This may result in a query
-                // using the default search engine.
-                if (TextUtils.isEmpty(keywordUrl)) {
+                // If there isn't a bookmark keyword, or if there is no search query
+                // within the keywordURL, yet one is provided, load the url.
+                // This may result in a query using the default search engine.
+                if (TextUtils.isEmpty(keywordUrl) ||
+                        (!TextUtils.isEmpty(keywordSearch) && !StringUtils.queryExists(keywordUrl))) {
                     Tabs.getInstance().loadUrl(url, Tabs.LOADURL_USER_ENTERED);
                     Telemetry.sendUIEvent(TelemetryContract.Event.LOAD_URL, TelemetryContract.Method.ACTIONBAR, "user");
                     return;
                 }
 
                 // Otherwise, construct a search query from the bookmark keyword.
                 // Replace lower case bookmark keywords with URLencoded search query or
                 // replace upper case bookmark keywords with un-encoded search query.
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/StringUtils.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/StringUtils.java
@@ -66,16 +66,28 @@ public class StringUtils {
         if (dot > -1 || colon > -1) {
             return false;
         }
         // Otherwise, text is ambiguous, and we keep its status unchanged
         return wasSearchQuery;
     }
 
     /**
+     * Check for the existence of %s and %S in a given URL
+     *
+     * @return True if  %s or %S exists, False otherwise.
+     */
+    public static boolean queryExists(final String inputURL) {
+        if (inputURL == null) {
+            return false;
+        }
+        return inputURL.contains("%s") || inputURL.contains("%S");
+    }
+
+    /**
      * Strip the ref from a URL, if present
      *
      * @return The base URL, without the ref. The original String is returned if it has no ref,
      *         of if the input is malformed.
      */
     public static String stripRef(final String inputURL) {
         if (inputURL == null) {
             return null;
@@ -313,9 +325,9 @@ public class StringUtils {
      * @param textPaint the paint used to render the text
      * @return          the width of the specified substring in screen pixels
      */
     public static int getTextWidth(final String text, final int start, final int end, final Paint textPaint) {
         final Rect bounds = new Rect();
         textPaint.getTextBounds(text, start, end, bounds);
         return bounds.width();
     }
-}
\ No newline at end of file
+}
--- a/mobile/android/geckoview/src/test/java/org/mozilla/gecko/util/TestStringUtils.java
+++ b/mobile/android/geckoview/src/test/java/org/mozilla/gecko/util/TestStringUtils.java
@@ -135,16 +135,35 @@ public class TestStringUtils {
         // test ambiguous
         String ambiguous = "~!@#$%^&*()_+`34567890-=qwertyuiop[]\\QWERTYUIOP{}|asdfghjkl;'ASDFGHJKL:\"ZXCVBNM<>?zxcvbnm,./";
         ambiguous = ambiguous.replace(" ","").replace(".","").replace(":","");
         assertTrue(StringUtils.isSearchQuery(ambiguous,true));
         assertFalse(StringUtils.isSearchQuery(ambiguous,false));
 
 
     }
+
+    @Test
+    public void testQueryExists(){
+        // test empty
+        assertFalse(StringUtils.queryExists(""));
+
+        // test single
+        assertFalse(StringUtils.queryExists("mozilla.org"));
+        assertFalse(StringUtils.queryExists("https://www.google.com/"));
+        assertTrue(StringUtils.queryExists("https://www.google.com/search?q=%s"));
+        assertTrue(StringUtils.queryExists("https://www.google.com/search?q=%S"));
+        assertTrue(StringUtils.queryExists("%s"));
+        assertTrue(StringUtils.queryExists("%S"));
+
+        //test double
+        assertTrue(StringUtils.queryExists("%s%S"));
+        assertTrue(StringUtils.queryExists("https://www.google.com/search?q=%s%S"));
+    }
+
     @Test
     public void testPathStartIndex(){
         // Tests without protocol
         assertTrue(StringUtils.pathStartIndex("mozilla.org") == -1);
         assertTrue(StringUtils.pathStartIndex("mozilla.org/en-US") == 11);
 
         // Tests with protocol
         assertTrue(StringUtils.pathStartIndex("https://mozilla.org") == -1);