Bug 1409803 - Copy logic to prevent spaces in keywords to new Edit Bookmark dialogue. r?jwu draft
authorJan Henning <jh+bugzilla@buttercookie.de>
Wed, 18 Oct 2017 22:34:14 +0200
changeset 689384 660ad824098ec0e4f7ef7eb1c328451e35aa312e
parent 689383 d4895f1b96e8b711ffff4ca7eee1b45095e85e66
child 689405 e201dcd143ba98e4fa1255aa15206d0607c92d1d
push id87007
push usermozilla@buttercookie.de
push dateTue, 31 Oct 2017 13:59:57 +0000
reviewersjwu
bugs1409803
milestone58.0a1
Bug 1409803 - Copy logic to prevent spaces in keywords to new Edit Bookmark dialogue. r?jwu MozReview-Commit-ID: JXF0zwxhVv4
mobile/android/base/java/org/mozilla/gecko/bookmarks/BookmarkEditFragment.java
--- a/mobile/android/base/java/org/mozilla/gecko/bookmarks/BookmarkEditFragment.java
+++ b/mobile/android/base/java/org/mozilla/gecko/bookmarks/BookmarkEditFragment.java
@@ -126,17 +126,17 @@ public class BookmarkEditFragment extend
         toolbar.inflateMenu(R.menu.bookmark_edit_menu);
         toolbar.setOnMenuItemClickListener(new Toolbar.OnMenuItemClickListener() {
             @Override
             public boolean onMenuItemClick(MenuItem item) {
                 switch (item.getItemId()) {
                     case R.id.done:
                         final String newUrl = locationText.getText().toString().trim();
                         final String newTitle = nameText.getText().toString();
-                        final String newKeyword = keywordText.getText().toString();
+                        final String newKeyword = keywordText.getText().toString().trim();
                         if (callbacks != null) {
                             if (TextUtils.equals(newTitle, bookmark.originalTitle) &&
                                 TextUtils.equals(newUrl, bookmark.originalUrl) &&
                                 TextUtils.equals(newKeyword, bookmark.originalKeyword) &&
                                 bookmark.parentId == bookmark.originalParentId) {
                                 // Nothing changed, skip callback.
                                 break;
                             }
@@ -255,24 +255,30 @@ public class BookmarkEditFragment extend
         } else {
             folderText.setText(bookmark.folder);
         }
 
         // Enable menu item after bookmark is set to view
         final MenuItem doneItem = toolbar.getMenu().findItem(R.id.done);
         doneItem.setEnabled(true);
 
-        // Add a TextWatcher to prevent invalid input(e.g. empty string).
+        // Add a TextWatcher to prevent invalid input (e.g. an empty string).
+        LocationTextWatcher locationTextWatcher = new LocationTextWatcher(doneItem);
+        KeywordTextWatcher keywordTextWatcher = new KeywordTextWatcher(doneItem);
+
+        // Cross reference the TextWatchers
+        locationTextWatcher.setPairedTextWatcher(keywordTextWatcher);
+        keywordTextWatcher.setPairedTextWatcher(locationTextWatcher);
+
         if (bookmark.type == Bookmarks.TYPE_FOLDER) {
-            BookmarkTextWatcher nameTextWatcher = new BookmarkTextWatcher(doneItem);
-            nameText.addTextChangedListener(nameTextWatcher);
+            nameText.addTextChangedListener(locationTextWatcher);
         } else {
-            BookmarkTextWatcher locationTextWatcher = new BookmarkTextWatcher(doneItem);
             locationText.addTextChangedListener(locationTextWatcher);
         }
+        keywordText.addTextChangedListener(keywordTextWatcher);
     }
 
     /**
      * A private struct to make it easier to pass bookmark data across threads
      */
     private static class Bookmark implements Parcelable {
         // Cannot be modified in this fragment.
         final long id;
@@ -504,42 +510,88 @@ public class BookmarkEditFragment extend
             // Ensure the loader is stopped.
             onStopLoading();
 
             bookmark = null;
         }
     }
 
     /**
-     * This text watcher enables the menu item if the dialog contains valid information, or disables otherwise.
+     * This text watcher watches to enable or disable the Save button if the dialog contains
+     * valid information. This class is overridden to do data checking on different fields.
+     * By itself, it always enables the button.
+     *
+     * Callers can also assign a paired partner to the TextWatcher, and callers will check
+     * that both are enabled before enabling the Save button.
      */
-    private static final class BookmarkTextWatcher implements TextWatcher {
-        // A stored reference to the dialog containing the text field being watched.
+    private static class EditBookmarkTextWatcher implements TextWatcher {
+        // A stored reference to the menu that should be disabled/enabled in response to the text
+        // being watched.
         private final WeakReference<MenuItem> doneItemWeakReference;
 
+        private EditBookmarkTextWatcher pairedTextWatcher;
+
         // Whether or not the menu item should be enabled.
-        private boolean enabled = true;
+        protected boolean enabled = true;
 
-        private BookmarkTextWatcher(MenuItem doneItem) {
+        private EditBookmarkTextWatcher(MenuItem doneItem) {
             doneItemWeakReference = new WeakReference<>(doneItem);
         }
 
+        public void setPairedTextWatcher(EditBookmarkTextWatcher textWatcher) {
+            pairedTextWatcher = textWatcher;
+        }
+
         public boolean isEnabled() {
             return enabled;
         }
 
         @Override
         public void onTextChanged(CharSequence s, int start, int before, int count) {
-            // Disables the menu item if the input field is empty.
-            final boolean enabled = (s.toString().trim().length() > 0);
+            // Disable if we're disabled or the paired partner is disabled.
+            boolean enabled = this.enabled && (pairedTextWatcher == null || pairedTextWatcher.isEnabled());
 
             final MenuItem doneItem = doneItemWeakReference.get();
             if (doneItem != null) {
                 doneItem.setEnabled(enabled);
             }
         }
 
         @Override
         public void afterTextChanged(Editable s) {}
         @Override
         public void beforeTextChanged(CharSequence s, int start, int count, int after) {}
     }
+
+    /**
+     * A version of the EditBookmarkTextWatcher for the URL field of the dialog.
+     * Only checks if the field is empty or not.
+     */
+    private static final class LocationTextWatcher extends EditBookmarkTextWatcher {
+        private LocationTextWatcher(MenuItem doneItem) {
+            super(doneItem);
+        }
+
+        @Override
+        public void onTextChanged(CharSequence s, int start, int before, int count) {
+            // Disables the menu item if the input field is empty.
+            enabled = (s.toString().trim().length() > 0);
+            super.onTextChanged(s, start, before, count);
+        }
+    }
+
+    /**
+     * A version of the EditBookmarkTextWatcher for the keyword field of the dialog.
+     * Checks if the field has any (non leading or trailing) spaces.
+     */
+    private static final class KeywordTextWatcher extends EditBookmarkTextWatcher {
+        private KeywordTextWatcher(MenuItem doneItem) {
+            super(doneItem);
+        }
+
+        @Override
+        public void onTextChanged(CharSequence s, int start, int before, int count) {
+            // Disable if the keyword contains spaces.
+            enabled = (s.toString().trim().indexOf(' ') == -1);
+            super.onTextChanged(s, start, before, count);
+        }
+    }
 }