Issue 2513 - Adblock Browser does not display chosen filter lists as active right away

mobile/android/thirdparty/org/adblockplus/browser/SubscriptionPreferenceCategory.java

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 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
@@ skipping 11 matching lines @@
 import org.mozilla.gecko.R;
 import org.mozilla.gecko.util.ThreadUtils;
 
 import android.app.ProgressDialog;
 import android.content.Context;
 import android.preference.CheckBoxPreference;
 import android.preference.Preference;
 import android.preference.PreferenceCategory;
 import android.util.AttributeSet;
 
-public class SubscriptionPreferenceCategory extends PreferenceCategory implements
-        Preference.OnPreferenceChangeListener, SubscriptionContainer.SubscriptionListener
+public class SubscriptionPreferenceCategory extends PreferenceCategory
 {
   volatile static SubscriptionContainer subscriptionContainer = null;
+  private final CheckBoxChangeListener checkBoxChangeListener = new CheckBoxChangeListener();
+  private final SubscriptionChangeListener subscriptionChangeListener = new SubscriptionChangeListener();
   private boolean isEnabledList = false;
   private ProgressDialog progressDialog;
 
   public SubscriptionPreferenceCategory(Context context)
   {
     super(context);
   }
 
   public SubscriptionPreferenceCategory(Context context, AttributeSet attrs)
   {
@@ skipping 28 matching lines @@
     super.onAttachedToActivity();
 
     showProgressDialog();
 
     AddOnBridge.postToHandler(new Runnable()
     {
       @Override
       public void run()
       {
         initSubscriptions();
-        subscriptionContainer.addSubscriptionListener(SubscriptionPreferenceCategory.this);
+        subscriptionContainer.addSubscriptionListener(
+            SubscriptionPreferenceCategory.this.subscriptionChangeListener);
+
         ThreadUtils.postToUiThread(new Runnable()
         {
           @Override
           public void run()
           {
             SubscriptionPreferenceCategory.this.refreshEntries();
           }
         });
       }
     });
   }
 
-  @Override
-  public boolean onPreferenceChange(Preference preference, Object newValue)
-  {
-    if (preference instanceof CheckBoxPreference && newValue instanceof Boolean)

[anton, 2016/09/30 07:12:53]

i'd prefer here to add spaces for each condition for better readability like
this:
`((preference instanceof CheckBoxPreference) && (newValue instanceof Boolean))`

not sure we have codestyle rule for it so it's just my suggestion

[diegocarloslima, 2016/10/26 13:04:33]

Actually, if you look below you will notice that this statement was already
present on this class just like this. Personally, I have no strong opinion about
adding parenthesis to this statement, but since it was already like this, I
think we should just keep it without them.

-    {
-      showProgressDialog();
-
-      final CheckBoxPreference cbp = (CheckBoxPreference) preference;
-      final boolean enable = ((Boolean) newValue).booleanValue();
-      SubscriptionPreferenceCategory.subscriptionContainer.changeSubscriptionState(
-              cbp.getKey(),
-              enable);
-    }
-    return true;
-  }
-
-  @Override
-  public void onSubscriptionUpdated()
-  {
-    refreshEntries();
-  }
-
   private CheckBoxPreference createDisabledCheckBox(final int titleId, final int summaryId)
   {
     final CheckBoxPreference cbp = new CheckBoxPreference(this.getContext());
     cbp.setTitle(titleId);
     cbp.setSummary(summaryId);
     cbp.setEnabled(false);
     cbp.setShouldDisableView(true);
     cbp.setSelectable(false);
     return cbp;
   }
 
   private CheckBoxPreference createEnabledCheckBox(
       final SubscriptionContainer.Subscription subscription)
   {
     final CheckBoxPreference cbp = new CheckBoxPreference(this.getContext());
     cbp.setTitle(subscription.specialization);
     cbp.setSummary(subscription.title);
     cbp.setChecked(true);
     cbp.setKey(subscription.url);
     cbp.setPersistent(false);
     cbp.setChecked(subscriptionContainer.isSubscriptionListed(subscription.url));
-    cbp.setOnPreferenceChangeListener(SubscriptionPreferenceCategory.this);
+    cbp.setOnPreferenceChangeListener(this.checkBoxChangeListener);
     return cbp;
   }
 
   private void refreshEntries()
   {
     if (this.isEnabledList)
     {
       this.refreshEntries(R.string.abb_adblocking_none_selected,
           R.string.abb_adblocking_select_below, true);
     }
@@ skipping 31 matching lines @@
   private void showProgressDialog()
   {
     this.progressDialog = new ProgressDialog(this.getContext());
     this.progressDialog.setProgressStyle(ProgressDialog.STYLE_SPINNER);
     this.progressDialog.setMessage(this.getContext().getString(R.string.abb_adblocking_waiting));
     this.progressDialog.show();
   }
 
   private void dismissProgressDialog()
   {
-    if(this.progressDialog != null)

[anton, 2016/09/30 07:12:53]

space here?

[diegocarloslima, 2016/10/26 13:04:33]

Acknowledged.

+    if (this.progressDialog != null)
     {
       this.progressDialog.dismiss();
       this.progressDialog = null;
     }
   }
+
+  private class CheckBoxChangeListener implements OnPreferenceChangeListener
+  {
+    @Override
+    public boolean onPreferenceChange(Preference preference, Object newValue)
+    {
+      if (preference instanceof CheckBoxPreference && newValue instanceof Boolean)
+      {
+        showProgressDialog();
+
+        final CheckBoxPreference cbp = (CheckBoxPreference) preference;
+        final boolean enable = ((Boolean) newValue).booleanValue();
+        SubscriptionPreferenceCategory.subscriptionContainer.changeSubscriptionState(
+            cbp.getKey(),
+            enable);
+      }
+      return true;
+    }
+  }
+
+  private class SubscriptionChangeListener implements SubscriptionContainer.SubscriptionListener
+  {
+    @Override
+    public void onSubscriptionUpdated()
+    {
+      SubscriptionPreferenceCategory.this.refreshEntries();
+    }
+  }
 }