Issue 3916 - Supporting Adding filter lists via URL

adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/InputValidatorDialogPreference.java

Index: adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/InputValidatorDialogPreference.java
===================================================================
rename from adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/UrlInputOpenerPreference.java
rename to adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/InputValidatorDialogPreference.java
--- a/adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/UrlInputOpenerPreference.java
+++ b/adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/InputValidatorDialogPreference.java
@@ -20,44 +20,54 @@ package org.adblockplus.sbrowser.content
 import android.app.AlertDialog;
 import android.content.Context;
 import android.os.Bundle;
 import android.preference.EditTextPreference;
 import android.text.Editable;
 import android.text.InputType;
 import android.text.TextWatcher;
 import android.util.AttributeSet;
+import android.util.Patterns;
 import android.view.KeyEvent;
 import android.view.View;
 import android.view.inputmethod.EditorInfo;
 import android.widget.EditText;
 import android.widget.TextView;
 import android.widget.Toast;
 
 import org.adblockplus.adblockplussbrowser.R;
 import org.adblockplus.sbrowser.contentblocker.util.PreferenceUtils;
 import org.apache.commons.validator.routines.DomainValidator;
 
-public class UrlInputOpenerPreference extends EditTextPreference implements TextWatcher,
+public class InputValidatorDialogPreference extends EditTextPreference implements TextWatcher,
     TextView.OnEditorActionListener
 {
 
-  private OnUrlReadyListener onUrlReadyListener;
-  private AlertDialog mAlertDialog;
-
-  public UrlInputOpenerPreference(Context context)
+  public enum ValidationType
   {
-    this(context, null);
+    DOMAIN,
+    URL
   }
 
-  public UrlInputOpenerPreference(Context context, AttributeSet attrs)
+  private OnInputReadyListener onInputReadyListener;
+  private AlertDialog mAlertDialog;

[diegocarloslima, 2017/09/22 17:43:42]

This variable passed through the last code review, but it should be renamed to
just 'alertDialog'. Since we're refactoring this class in this issue, I'm fine
with either changing it here or in a follow up issue. What do you think?

[jens, 2017/09/26 10:25:01]

For me it's totally fine to change the name of the variable within this ticket.

+  private ValidationType validationType;
+
+  public InputValidatorDialogPreference(Context context, final ValidationType validationType)
+  {
+    this(context, null, validationType);
+  }
+
+  public InputValidatorDialogPreference(Context context, AttributeSet attrs,
+      final ValidationType validationType)

[diegocarloslima, 2017/09/22 17:43:42]

This type of constructor will fail if the class is placed into a xml file,
because of the ValidationType argument

[jens, 2017/09/26 10:25:01]

Oh, good catch. Should we create a separate ticket to make it possible to set
ValidationType from XML? For now I would suggest to move setting the defaults to
the three arguments default constructor and remove validationType from this one.

   {
     super(context, attrs);
 
     // Setting defaults
+    this.validationType = validationType;
     this.setIcon(android.R.drawable.ic_menu_add);
     this.setPositiveButtonText(android.R.string.ok);
     this.setNegativeButtonText(android.R.string.cancel);
     final EditText editText = getEditText();
     editText.addTextChangedListener(this);
     editText.setOnEditorActionListener(this);
     editText.setInputType(InputType.TYPE_TEXT_VARIATION_URI);
   }
@@ -73,19 +83,19 @@ public class UrlInputOpenerPreference ex
   }
 
   @Override
   protected void onDialogClosed(boolean positiveResult)
   {
     super.onDialogClosed(positiveResult);
 
     mAlertDialog = null;
-    if (positiveResult && this.onUrlReadyListener != null)
+    if (positiveResult && this.onInputReadyListener != null)
     {
-      this.onUrlReadyListener.onUrlReady(getUrl());
+      this.onInputReadyListener.onInputReady(getUrl());
     }
   }
 
   @Override
   public void beforeTextChanged(CharSequence s, int start, int count, int after)
   {
     // Ignored
   }
@@ -94,25 +104,25 @@ public class UrlInputOpenerPreference ex
   public void onTextChanged(CharSequence s, int start, int before, int count)
   {
     // Ignored
   }
 
   @Override
   public void afterTextChanged(Editable s)
   {
-    setPositiveButtonEnabled(isValidDomain());
+    setPositiveButtonEnabled(isValidInput());
   }
 
   @Override
   public boolean onEditorAction(TextView v, int actionId, KeyEvent event)
   {
     if (actionId == EditorInfo.IME_ACTION_DONE)
     {
-      if (this.isValidDomain())
+      if (this.isValidInput())
       {
         mAlertDialog.getButton(AlertDialog.BUTTON_POSITIVE).performClick();
       }
       else
       {
         Toast.makeText(getContext(), R.string.whitelist_website_invalid_url_error, Toast.LENGTH_SHORT).show();

[diegocarloslima, 2017/09/22 17:43:42]

We might consider making possible to change the text of the error message
externally, by defining a setErrorMessage() method

[jens, 2017/09/26 10:25:01]

o have different error messages for different ValidationTypes? That's a good
idea but maybe we should do it in a separate ticket as we will need new strings
ans translations too.

       }
       return true;
@@ -122,36 +132,52 @@ public class UrlInputOpenerPreference ex
 
   @Override
   protected void onBindView(View view)
   {
     super.onBindView(view);
     PreferenceUtils.setMultilineTitle(view);
   }
 
-  public void setOnUrlReadyListener(OnUrlReadyListener listener)
+  public void setOnInputReadyListener(OnInputReadyListener listener)
   {
-    this.onUrlReadyListener = listener;
+    this.onInputReadyListener = listener;
   }
 
   private boolean isValidDomain()
   {
     return DomainValidator.getInstance().isValid(getUrl());
   }
 
+  private boolean isValidUrl()
+  {
+    return Patterns.WEB_URL.matcher(getUrl()).matches();
+  }
+
+  private boolean isValidInput()
+  {
+    switch (validationType)
+    {
+      case DOMAIN:
+        return isValidDomain();
+      default:
+        return isValidUrl();
+    }
+  }
+
   private String getUrl()

[diegocarloslima, 2017/09/22 17:43:42]

This might be renamed to getInput()

[jens, 2017/09/26 10:25:01]

Acknowledged.

   {
     return getEditText().getText().toString();
   }
 
   private void setPositiveButtonEnabled(boolean enabled)
   {
     if (mAlertDialog != null)
     {
       mAlertDialog.getButton(AlertDialog.BUTTON_POSITIVE).setEnabled(enabled);
     }
   }
 
-  public interface OnUrlReadyListener
+  public interface OnInputReadyListener
   {
-    void onUrlReady(String url);
+    void onInputReady(String url);

[diegocarloslima, 2017/09/22 17:43:41]

I would suggest changing from `String url` to `String input`

[jens, 2017/09/26 10:25:01]

Acknowledged.

   }
-}
+}