Bug 1409803 - Copy logic to prevent spaces in keywords to new Edit Bookmark dialogue. r=jwu
authorJan Henning <jh+bugzilla@buttercookie.de>
Wed, 18 Oct 2017 22:34:14 +0200
changeset 389426 41625f3d3a6af421e4a837a633e623b491208614
parent 389425 adf78145b388b8611ff4ce4741522b454a418e35
child 389427 c0ade1e797372449e3dbbb02168ddb411cdf1f66
push id96855
push userarchaeopteryx@coole-files.de
push dateTue, 31 Oct 2017 23:40:37 +0000
treeherdermozilla-inbound@285362745f60 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwu
bugs1409803
milestone58.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 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);
+        }
+    }
 }