Create a minimal settings UI

mobile/android/thirdparty/org/adblockplus/browser/AbpCheckBoxPreference.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
  * 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.browser;
 
 import org.mozilla.gecko.preferences.CustomCheckBoxPreference;
 import org.mozilla.gecko.util.NativeJSObject;
 import org.mozilla.gecko.util.ThreadUtils;
 
 import android.content.Context;
 import android.util.AttributeSet;
 
 public class AbpCheckBoxPreference extends CustomCheckBoxPreference implements

[Felix Dahlke, 2015/03/22 15:51:45]

What's that, 80 columns? :P We do have 100 for Java code.

[René Jeschke, 2015/03/22 16:44:55]

Yes, and it would end on column 102 :p

[Felix Dahlke, 2015/03/22 17:44:34]

Touché :P

     AdblockPlusApiCallback
 {
   private String apiKey = "";
 
   public AbpCheckBoxPreference(Context context)
   {
     super(context);
   }
 
   public AbpCheckBoxPreference(Context context, AttributeSet attrs)
   {
     super(context, attrs);
   }
 
   public AbpCheckBoxPreference(Context context, AttributeSet attrs, int defStyle)
   {
     super(context, attrs, defStyle);
   }
 
   @Override
   protected void onAttachedToActivity()
   {
     super.onAttachedToActivity();
-    this.apiKey = this.getKey().substring(this.getKey().lastIndexOf('.') + 1);
+    this.apiKey = this.getKey().substring(this.getKey().lastIndexOf(".api") + 4);
     this.setEnabled(false);
-    this.recheckValue();
+    this.refreshValue();
   }
 
-  private void recheckValue()

[Felix Dahlke, 2015/03/22 15:51:45]

Naming nit: Isn't this rather logically "refreshValue"?

[René Jeschke, 2015/03/22 16:44:55]

Yup

+  private void refreshValue()
   {
-    AddOnBridge.queryBoolean(this, "query_" + this.apiKey);
+    AddOnBridge.queryBoolean(this, this.apiKey);
   }
 
   @Override
   protected void onClick()

[Felix Dahlke, 2015/03/22 15:51:45]

IIRC, setChecked is invoked whenever the value changes (thus including
super.onClick) - so how about overwriting that instead?

[René Jeschke, 2015/03/22 16:44:55]

Because I call 'setChecked()' to set the checkbox checked, and I want it to
change when the user 'clicks' it.

[Felix Dahlke, 2015/03/22 17:44:34]

Well, you could use super.setChecked. Potentially problematic here is that if
the checkbox is somehow checked without clicking it, we'll miss it.

[René Jeschke, 2015/03/22 18:32:41]

What I mean is:

If we do put this logic into 'setChecked()' then we can never ever change the
value, because the second this component gets initialized, it will read it's
default value and call 'setChecked()', which then makes the value persistent by
sending it to ABP.

onAttachToActivity() fetches the current value from ABP and calls 'setChecked()'
to reflect this value onto our checkbox.

From this moment on, we're displayed as a UI element ... and the only way this
will get changed is by 'clicking'.

'setChecked' does not work, as we can not figure out who is calling us and if we
need to ignore it or not.

[Felix Dahlke, 2015/03/22 20:10:24]

You mean, CheckBoxPreference itself will call setChecked on itself initially?
That's problematic of course, if that's the case we probably have to go for
onClick.
Can't we effectively call super.setChecked in onApiRequestSucceeded?
 
So far the theory :) Would arguably be more robust to support setChecked
properly.

[René Jeschke, 2015/03/22 21:04:45]

Ok, as discussed on IRC, we will leave this as is.

   {
     super.onClick();
-    AddOnBridge.setBoolean(this, "change_" + this.apiKey, this.isChecked());
-    this.recheckValue();
+    AddOnBridge.setBoolean(this, this.apiKey, this.isChecked());
+    this.refreshValue();
   }
 
   @Override
   public void onApiRequestSucceeded(NativeJSObject jsObject)
   {
     if (jsObject.has("value"))
     {
-      final boolean enabled = AddOnBridge.getBooleanFromJSObject(jsObject, "value", false);
+      final boolean enabled = AddOnBridge.getBooleanFromJsObject(jsObject, "value", false);
 
       ThreadUtils.postToUiThread(new Runnable()
       {
         @Override
         public void run()
         {
           AbpCheckBoxPreference.this.setChecked(enabled);
           AbpCheckBoxPreference.this.setEnabled(true);
         }
       });
     }
   }
 
   @Override
   public void onApiRequestFailed(String errorMessage)
   {
     // Currently ignored
   }
 }