Issue 3916 - Supporting Adding filter lists via URL

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

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 package org.adblockplus.sbrowser.contentblocker;
 
 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);
   }
 
   @Override
   protected void showDialog(Bundle state)
   {
     super.showDialog(state);
 
     mAlertDialog = (AlertDialog) getDialog();
     // Positive button is disabled until a valid URL is entered
     this.setPositiveButtonEnabled(false);
   }
 
   @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
   }
 
   @Override
   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;
     }
     return false;
   }
 
   @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.

   }
-}
+}