Issue 3916 - Supporting Adding filter lists via URL

adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/UrlInputOpenerPreference.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,

[diegocarloslima, 2017/09/08 15:06:07]

I would suggest to rename the class to something like InputOpenerPreference or
InputValidatorDialogPreference, since now it became a more general purpose class
that can validate URLs and domain (and can validate other stuff in the future).

[jens, 2017/09/13 08:50:29]

Acknowledged.

     TextView.OnEditorActionListener
 {
 
   private OnUrlReadyListener onUrlReadyListener;
   private AlertDialog mAlertDialog;
+  private boolean validateTld;

[anton, 2017/08/25 10:40:40]

sorry, what is `Tld` (did i miss anything)? Can you change it to anything more
readable?

[jens, 2017/08/25 11:35:06]

It stands for top-level domain. I thought it is a common abbreviation. The
reason I used this name is, that we introduced a domain validation to ensure
that only  existing top-level domains can be entered when whitelisting a
website. I will change the name to 'validateTopLevelDomain'

[anton, 2017/08/25 11:43:30]

Okay, why not just call it 'validateDomain' for simplicity and add a comment
that it validates only TLD?
We can ask Diego what he thinks about it.

"There are only two hard things in Computer Science: cache invalidation and
naming things" ;)

[diegocarloslima, 2017/09/08 15:06:07]

I would suggest to create an enum for that, so we can extend this preference to
validate other types of inputs in the future.
I would call the enum ValidationType {DOMAIN, URL}

[jens, 2017/09/13 08:50:29]

Acknowledged.

 
-  public UrlInputOpenerPreference(Context context)
+  public UrlInputOpenerPreference(Context context, final boolean validateTld)
   {
-    this(context, null);
+    this(context, null, validateTld);
   }
 
-  public UrlInputOpenerPreference(Context context, AttributeSet attrs)
+  public UrlInputOpenerPreference(Context context, AttributeSet attrs, final boolean validateTld)
   {
     super(context, attrs);
 
     // Setting defaults
+    this.validateTld = validateTld;
     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
@@ skipping 26 matching lines @@
 
   @Override
   public void onTextChanged(CharSequence s, int start, int before, int count)
   {
     // Ignored
   }
 
   @Override
   public void afterTextChanged(Editable s)
   {
-    setPositiveButtonEnabled(isValidDomain());
+    setPositiveButtonEnabled(validate());
   }
 
   @Override
   public boolean onEditorAction(TextView v, int actionId, KeyEvent event)
   {
     if (actionId == EditorInfo.IME_ACTION_DONE)
     {
-      if (this.isValidDomain())
+      if (this.validate())
       {
         mAlertDialog.getButton(AlertDialog.BUTTON_POSITIVE).performClick();
       }
       else
       {
         Toast.makeText(getContext(), R.string.whitelist_website_invalid_url_error, Toast.LENGTH_SHORT).show();
       }
       return true;
     }
     return false;
   }
 
   @Override
   protected void onBindView(View view)
   {
     super.onBindView(view);
     PreferenceUtils.setMultilineTitle(view);
   }
 
   public void setOnUrlReadyListener(OnUrlReadyListener listener)
   {
     this.onUrlReadyListener = listener;
   }
 
   private boolean isValidDomain()
   {
     return DomainValidator.getInstance().isValid(getUrl());
   }
 
+  private boolean isValidUrl()
+  {
+    return Patterns.WEB_URL.matcher(getUrl()).matches();
+  }
+
+  private boolean validate()

[anton, 2017/08/25 10:40:40]

improvement suggestion: `validate` makes me think it's just an action (without
result)
and `isValid()` sounds better for me. Also you already have 'isValidUrl' and
'isValidDomain' methods.

[jens, 2017/08/25 11:35:06]

Acknowledged.

[diegocarloslima, 2017/09/08 15:06:07]

I would further suggest to rename it to isValidInput()

[jens, 2017/09/13 08:50:30]

Acknowledged.

+  {
+    if (validateTld)
+    {
+      return isValidDomain();
+    }
+    else
+    {
+      return isValidUrl();
+    }
+  }
+
   private String getUrl()
   {
     return getEditText().getText().toString();
   }
 
   private void setPositiveButtonEnabled(boolean enabled)
   {
     if (mAlertDialog != null)
     {
       mAlertDialog.getButton(AlertDialog.BUTTON_POSITIVE).setEnabled(enabled);
     }
   }
 
   public interface OnUrlReadyListener

[diegocarloslima, 2017/09/08 15:06:07]

I would also suggest to rename the listener to OnInputReadyListener and the
method to onInputReady(String input)

[jens, 2017/09/13 08:50:30]

Acknowledged.

   {
     void onUrlReady(String url);
   }
-}
+}