Issue mozilla-mobile#3647 - Bring back siteSecurityColor

NotWoods · Aug 12, 2019 · d94edf9 · d94edf9
1 parent 56cbebd
commit d94edf9
Show file tree

Hide file tree

Showing 14 changed files with 195 additions and 204 deletions.
diff --git a/...onents/browser/toolbar/src/main/java/mozilla/components/browser/toolbar/BrowserToolbar.kt b/...onents/browser/toolbar/src/main/java/mozilla/components/browser/toolbar/BrowserToolbar.kt
@@ -31,7 +31,6 @@ import kotlinx.coroutines.launch
 import mozilla.components.browser.menu.BrowserMenuBuilder
 import mozilla.components.browser.toolbar.display.DisplayToolbar
 import mozilla.components.browser.toolbar.display.DisplayToolbar.Companion.BOTTOM_PROGRESS_BAR
-import mozilla.components.browser.toolbar.display.SiteSecurityIcons
 import mozilla.components.browser.toolbar.display.TrackingProtectionIconView
 import mozilla.components.browser.toolbar.edit.EditToolbar
 import mozilla.components.concept.toolbar.AutocompleteDelegate
@@ -114,12 +113,21 @@ class BrowserToolbar @JvmOverloads constructor(
         }
 
     /**
-     *  Set/Get the site security icons (usually a lock or globe icon). It uses a pair of drawables
-     *  which represent the insecure and secure colours respectively.
+     * Set/Get the site security icon (usually a lock and globe icon). It uses a
+     * [android.graphics.drawable.StateListDrawable] where "state_site_secure" represents the secure
+     * icon and empty state represents the insecure icon.
      */
-    var siteSecurityIcons
-        get() = displayToolbar.securityIcons
-        set(value) { displayToolbar.securityIcons = value }
+    var siteSecurityIcon
+        get() = displayToolbar.securityIcon
+        set(value) { displayToolbar.securityIcon = value }
+
+    /**
+     * Set/Get the site security icon colours. It uses a pair of color integers
+     * which represent the insecure and secure colours respectively.
+     */
+    var siteSecurityColor: Pair<Int, Int>
+        get() = displayToolbar.securityIconColor
+        set(value) { displayToolbar.securityIconColor = value }
 
     /**
      * Gets/Sets a custom view that will be drawn as behind the URL and page actions in display mode.
@@ -301,11 +309,6 @@ class BrowserToolbar @JvmOverloads constructor(
             editToolbar.urlView.typeface = value
         }
 
-    fun setSiteSecurityColor(colors: Pair<Int, Int>) {
-        displayToolbar.securityIcons =
-            displayToolbar.securityIcons.withColorFilter(colors.first, colors.second)
-    }
-
     /**
      * Sets a listener to be invoked when focus of the URL input view (in edit mode) changed.
      */
@@ -474,20 +477,17 @@ class BrowserToolbar @JvmOverloads constructor(
                     R.styleable.BrowserToolbar_browserToolbarSuggestionBackgroundColor,
                     suggestionBackgroundColor
                 )
-                val inSecureIcon = getDrawable(R.styleable.BrowserToolbar_browserToolbarInsecureIcon)
-                    ?: displayToolbar.securityIcons.insecure
-                val secureIcon = getDrawable(R.styleable.BrowserToolbar_browserToolbarSecureIcon)
-                    ?: displayToolbar.securityIcons.secure
-                val inSecureColor = getColor(
+                siteSecurityIcon = getDrawable(R.styleable.BrowserToolbar_browserToolbarSecurityIcon)
+                    ?: displayToolbar.securityIcon
+                val insecureColor = getColor(
                     R.styleable.BrowserToolbar_browserToolbarInsecureColor,
                     displayToolbar.defaultColor
                 )
                 val secureColor = getColor(
-                    R.styleable.BrowserToolbar_browserToolbarSecureColor,
+                    R.styleable.BrowserToolbar_browserToolbarInsecureColor,
                     displayToolbar.defaultColor
                 )
-                siteSecurityIcons = SiteSecurityIcons(inSecureIcon, secureIcon)
-                    .withColorFilter(inSecureColor, secureColor)
+                siteSecurityColor = insecureColor to secureColor
                 val fadingEdgeLength = getDimensionPixelSize(
                     R.styleable.BrowserToolbar_browserToolbarFadingEdgeSize,
                     resources.getDimensionPixelSize(R.dimen.mozac_browser_toolbar_url_fading_edge_size)

diff --git a/...rowser/toolbar/src/main/java/mozilla/components/browser/toolbar/display/DisplayToolbar.kt b/...rowser/toolbar/src/main/java/mozilla/components/browser/toolbar/display/DisplayToolbar.kt
@@ -6,6 +6,7 @@ package mozilla.components.browser.toolbar.display
 
 import android.annotation.SuppressLint
 import android.content.Context
+import android.graphics.Color
 import android.graphics.drawable.Drawable
 import android.view.Gravity
 import android.view.View
@@ -29,10 +30,10 @@ import mozilla.components.browser.toolbar.internal.wrapAction
 import mozilla.components.concept.toolbar.Toolbar
 import mozilla.components.concept.toolbar.Toolbar.SiteSecurity
 import mozilla.components.concept.toolbar.Toolbar.SiteTrackingProtection
+import mozilla.components.concept.toolbar.Toolbar.SiteTrackingProtection.OFF_FOR_A_SITE
+import mozilla.components.concept.toolbar.Toolbar.SiteTrackingProtection.OFF_GLOBALLY
 import mozilla.components.concept.toolbar.Toolbar.SiteTrackingProtection.ON_NO_TRACKERS_BLOCKED
 import mozilla.components.concept.toolbar.Toolbar.SiteTrackingProtection.ON_TRACKERS_BLOCKED
-import mozilla.components.concept.toolbar.Toolbar.SiteTrackingProtection.OFF_GLOBALLY
-import mozilla.components.concept.toolbar.Toolbar.SiteTrackingProtection.OFF_FOR_A_SITE
 
 /**
  * Sub-component of the browser toolbar responsible for displaying the URL and related controls.
@@ -127,7 +128,13 @@ internal class DisplayToolbar(
 
     private var siteTrackingProtection = OFF_GLOBALLY
 
-    internal var securityIcons = SiteSecurityIcons.getDefaultSecurityIcons(context, defaultColor)
+    internal var securityIcon = context.getDrawable(R.drawable.mozac_ic_site_security)
+        set(value) {
+            field = value
+            siteSecurityIconView.setImageDrawable(value)
+        }
+
+    internal var securityIconColor = defaultColor to defaultColor
         set(value) {
             field = value
             setSiteSecurity(currentSiteSecurity)
@@ -160,11 +167,9 @@ internal class DisplayToolbar(
             setOnClickListener(null)
         }
 
-    internal val siteSecurityIconView = AppCompatImageView(context).apply {
+    internal val siteSecurityIconView = SiteSecurityIconView(context).apply {
         setPadding(resources.getDimensionPixelSize(R.dimen.mozac_browser_toolbar_icon_padding))
 
-        setImageDrawable(securityIcons.insecure)
-
         // Avoiding text behind the icon being selectable. If the listener is not set
         // with a value or null text behind the icon can be selectable.
         // https://github.com/mozilla-mobile/reference-browser/issues/448
@@ -305,12 +310,17 @@ internal class DisplayToolbar(
      * Sets the site's security icon as secure if true, else the regular globe.
      */
     fun setSiteSecurity(secure: SiteSecurity) {
-        val drawable = when (secure) {
-            SiteSecurity.INSECURE -> securityIcons.insecure
-            SiteSecurity.SECURE -> securityIcons.secure
+        @ColorInt val color = when (secure) {
+            SiteSecurity.INSECURE -> securityIconColor.first
+            SiteSecurity.SECURE -> securityIconColor.second
+        }
+        if (color == Color.TRANSPARENT) {
+            siteSecurityIconView.clearColorFilter()
+        } else {
+            siteSecurityIconView.setColorFilter(color)
         }
-        siteSecurityIconView.setImageDrawable(drawable)
 
+        siteSecurityIconView.siteSecurity = secure
         currentSiteSecurity = secure
     }
 

diff --git a/.../toolbar/src/main/java/mozilla/components/browser/toolbar/display/SiteSecurityIconView.kt b/.../toolbar/src/main/java/mozilla/components/browser/toolbar/display/SiteSecurityIconView.kt
@@ -0,0 +1,48 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package mozilla.components.browser.toolbar.display
+
+import android.content.Context
+import android.util.AttributeSet
+import android.view.View
+import androidx.appcompat.widget.AppCompatImageView
+import mozilla.components.browser.toolbar.R
+import mozilla.components.concept.toolbar.Toolbar.SiteSecurity
+
+/**
+ * Internal widget to display the different icons of site security, relies on the
+ * [SiteSecurity] state of each page.
+ */
+internal class SiteSecurityIconView @JvmOverloads constructor(
+    context: Context,
+    attrs: AttributeSet? = null,
+    defStyleAttr: Int = 0
+) : AppCompatImageView(context, attrs, defStyleAttr) {
+
+    // We allow null here because in some situations, onCreateDrawableState is getting called from
+    // the super() constructor on the View class, way before this class properties get
+    // initialized causing a null pointer exception.
+    // See for more details: https://github.com/mozilla-mobile/android-components/issues/4058
+    var siteSecurity: SiteSecurity? = SiteSecurity.INSECURE
+        set(value) {
+            if (value != field) {
+                field = value
+                refreshDrawableState()
+            }
+
+            field = value
+        }
+
+    override fun onCreateDrawableState(extraSpace: Int): IntArray {
+        return when (siteSecurity) {
+            SiteSecurity.INSECURE, null -> super.onCreateDrawableState(extraSpace)
+            SiteSecurity.SECURE -> {
+                val drawableState = super.onCreateDrawableState(extraSpace + 1)
+                View.mergeDrawableStates(drawableState, intArrayOf(R.attr.state_site_secure))
+                drawableState
+            }
+        }
+    }
+}
diff --git a/...ser/toolbar/src/main/java/mozilla/components/browser/toolbar/display/SiteSecurityIcons.kt b/...ser/toolbar/src/main/java/mozilla/components/browser/toolbar/display/SiteSecurityIcons.kt
diff --git a/components/browser/toolbar/src/main/res/drawable/mozac_ic_site_security.xml b/components/browser/toolbar/src/main/res/drawable/mozac_ic_site_security.xml
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- This Source Code Form is subject to the terms of the Mozilla Public
+   - License, v. 2.0. If a copy of the MPL was not distributed with this
+   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
+<selector xmlns:android="http://schemas.android.com/apk/res/android" xmlns:ac="http://schemas.android.com/apk/res-auto">
+    <item android:drawable="@drawable/mozac_ic_lock" ac:state_site_secure="true" />
+    <item android:drawable="@drawable/mozac_ic_globe" />
+</selector>
diff --git a/components/browser/toolbar/src/main/res/values/attrs_browser_toolbar.xml b/components/browser/toolbar/src/main/res/values/attrs_browser_toolbar.xml
@@ -8,10 +8,9 @@
     <attr name="browserToolbarHintColor" format="color"/>
     <attr name="browserToolbarTextColor" format="color"/>
     <attr name="browserToolbarTextSize" format="dimension"/>
-    <attr name="browserToolbarSecureIcon" format="reference"/>
-    <attr name="browserToolbarInsecureIcon" format="reference"/>
-    <attr name="browserToolbarSecureColor" format="color"/>
+    <attr name="browserToolbarSecurityIcon" format="reference"/>
     <attr name="browserToolbarInsecureColor" format="color"/>
+    <attr name="browserToolbarSecureColor" format="color"/>
     <attr name="browserToolbarMenuColor" format="color"/>
     <attr name="browserToolbarSuggestionBackgroundColor" format="color" />
     <attr name="browserToolbarSuggestionForegroundColor" format="color" />
@@ -29,4 +28,8 @@
     <attr name="stateOnNoTrackersBlocked" format="boolean"/>
     <attr name="stateOffForASite" format="boolean"/>
   </declare-styleable>
+
+  <declare-styleable name="BrowserToolbarSiteSecurityState">
+    <attr name="state_site_secure" format="boolean"/>
+  </declare-styleable>
 </resources>
diff --git a/...ts/browser/toolbar/src/test/java/mozilla/components/browser/toolbar/BrowserToolbarTest.kt b/...ts/browser/toolbar/src/test/java/mozilla/components/browser/toolbar/BrowserToolbarTest.kt
@@ -6,8 +6,6 @@ package mozilla.components.browser.toolbar
 
 import android.content.Context
 import android.graphics.Color
-import android.graphics.PorterDuff
-import android.graphics.PorterDuffColorFilter
 import android.graphics.Typeface
 import android.graphics.drawable.Drawable
 import android.util.AttributeSet
@@ -908,23 +906,6 @@ class BrowserToolbarTest {
         assertEquals(View.VISIBLE, toolbar.displayToolbar.siteSecurityIconView.visibility)
     }
 
-    @Test
-    fun `siteSecurityColor setter`() {
-        val toolbar = BrowserToolbar(testContext)
-
-        toolbar.setSiteSecurityColor(Color.RED to Color.BLUE)
-        assertEquals(
-            PorterDuffColorFilter(Color.RED, PorterDuff.Mode.SRC_IN),
-            toolbar.displayToolbar.siteSecurityIconView.drawable.colorFilter
-        )
-
-        toolbar.siteSecure = SiteSecurity.SECURE
-        assertEquals(
-            PorterDuffColorFilter(Color.BLUE, PorterDuff.Mode.SRC_IN),
-            toolbar.displayToolbar.siteSecurityIconView.drawable.colorFilter
-        )
-    }
-
     @Test
     fun `urlBoxView getter`() {
         val toolbar = BrowserToolbar(testContext)