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

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

Index: mobile/android/thirdparty/org/adblockplus/browser/SubscriptionPreferenceCategory.java
===================================================================
--- a/mobile/android/thirdparty/org/adblockplus/browser/SubscriptionPreferenceCategory.java
+++ b/mobile/android/thirdparty/org/adblockplus/browser/SubscriptionPreferenceCategory.java
@@ -24,20 +24,20 @@ import org.mozilla.gecko.util.ThreadUtil
 
 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
+public class SubscriptionPreferenceCategory extends PreferenceCategory implements
+        Preference.OnPreferenceChangeListener, SubscriptionContainer.SubscriptionListener
 {
   volatile static SubscriptionContainer subscriptionContainer = null;
-  private final CheckBoxChangeListener checkBoxChangeListener = new CheckBoxChangeListener();
   private boolean isEnabledList = false;
   private ProgressDialog progressDialog;
 
   public SubscriptionPreferenceCategory(Context context)
   {
     super(context);
   }
 
@@ -63,45 +63,64 @@ public class SubscriptionPreferenceCateg
     }
   }
 
   @Override
   protected void onAttachedToActivity()
   {
     this.isEnabledList = this.getKey().endsWith("subscriptionEnabled");
 
-    this.progressDialog = new ProgressDialog(this.getContext());
-
     this.setEnabled(false);
     this.setShouldDisableView(true);
 
     super.onAttachedToActivity();
 
-    this.progressDialog.setProgressStyle(ProgressDialog.STYLE_SPINNER);
-    this.progressDialog.setMessage(this.getContext().getString(R.string.abb_adblocking_waiting));
-    this.progressDialog.show();
+    showProgressDialog();
 
     AddOnBridge.postToHandler(new Runnable()
     {
       @Override
       public void run()
       {
         initSubscriptions();
+        subscriptionContainer.addSubscriptionListener(SubscriptionPreferenceCategory.this);
         ThreadUtils.postToUiThread(new Runnable()
         {
           @Override
           public void run()
           {
-            SubscriptionPreferenceCategory.this.initEntries();
+            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);
@@ -113,40 +132,40 @@ public class SubscriptionPreferenceCateg
   {
     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(this.checkBoxChangeListener);
+    cbp.setOnPreferenceChangeListener(SubscriptionPreferenceCategory.this);
     return cbp;
   }
 
-  private void initEntries()
+  private void refreshEntries()
   {
     if (this.isEnabledList)
     {
-      this.initEntries(R.string.abb_adblocking_none_selected,
+      this.refreshEntries(R.string.abb_adblocking_none_selected,
           R.string.abb_adblocking_select_below, true);
     }
     else
     {
-      this.initEntries(R.string.abb_adblocking_none_available,
+      this.refreshEntries(R.string.abb_adblocking_none_available,
           R.string.abb_adblocking_all_selected, false);
     }
 
     this.setEnabled(true);
     this.setShouldDisableView(false);
-    this.progressDialog.dismiss();
-    this.progressDialog = null;
+
+    dismissProgressDialog();
   }
 
-  private void initEntries(final int titleId, final int summaryId, boolean enabled)
+  private void refreshEntries(final int titleId, final int summaryId, boolean enabled)
   {
     this.removeAll();
     final List<SubscriptionContainer.Subscription> entries =
         subscriptionContainer.getSubscriptions(enabled);
 
     if (entries.isEmpty())
     {
       this.addPreference(this.createDisabledCheckBox(titleId, summaryId));
@@ -155,26 +174,26 @@ public class SubscriptionPreferenceCateg
     {
       for (SubscriptionContainer.Subscription e : entries)
       {
         this.addPreference(this.createEnabledCheckBox(e));
       }
     }
   }
 
-  private static class CheckBoxChangeListener implements OnPreferenceChangeListener
+  private void showProgressDialog()
   {
-    @Override
-    public boolean onPreferenceChange(Preference preference, Object newValue)
+    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 (preference instanceof CheckBoxPreference && newValue instanceof Boolean)
-      {
-        final CheckBoxPreference cbp = (CheckBoxPreference) preference;
-        final boolean enable = ((Boolean) newValue).booleanValue();
-        SubscriptionPreferenceCategory.subscriptionContainer.changeSubscriptionState(
-            cbp.getKey(),
-            enable);
-      }
-      return true;
+      this.progressDialog.dismiss();
+      this.progressDialog = null;
     }
   }
 }