From 02fb0aa124b3c0ec9879ba8c5373857ccddbcb04 Mon Sep 17 00:00:00 2001 From: Tiger Oakes Date: Fri, 2 Aug 2019 14:18:22 -0400 Subject: [PATCH] Issue #3647 - Custom HTTP(S) icons in toolbar --- .../browser/toolbar/BrowserToolbar.kt | 23 ++-- .../browser/toolbar/display/DisplayToolbar.kt | 119 +++++++++--------- .../toolbar/display/SiteSecurityIcons.kt | 61 +++++++++ .../main/res/values/attrs_browser_toolbar.xml | 4 +- .../toolbar/display/DisplayToolbarTest.kt | 29 +++-- .../browser/toolbar/display/MenuButtonTest.kt | 19 +-- .../toolbar/display/SiteSecurityIconsTest.kt | 55 ++++++++ .../customtabs/CustomTabsToolbarFeature.kt | 2 +- docs/changelog.md | 7 +- 9 files changed, 221 insertions(+), 98 deletions(-) create mode 100644 components/browser/toolbar/src/main/java/mozilla/components/browser/toolbar/display/SiteSecurityIcons.kt create mode 100644 components/browser/toolbar/src/test/java/mozilla/components/browser/toolbar/display/SiteSecurityIconsTest.kt diff --git a/components/browser/toolbar/src/main/java/mozilla/components/browser/toolbar/BrowserToolbar.kt b/components/browser/toolbar/src/main/java/mozilla/components/browser/toolbar/BrowserToolbar.kt index 6c79c1b9f07..1ebe7561b98 100644 --- a/components/browser/toolbar/src/main/java/mozilla/components/browser/toolbar/BrowserToolbar.kt +++ b/components/browser/toolbar/src/main/java/mozilla/components/browser/toolbar/BrowserToolbar.kt @@ -32,6 +32,7 @@ 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.TrackingProtectionIconView +import mozilla.components.browser.toolbar.display.SiteSecurityIcons import mozilla.components.browser.toolbar.edit.EditToolbar import mozilla.components.concept.toolbar.AutocompleteDelegate import mozilla.components.concept.toolbar.AutocompleteResult @@ -113,12 +114,12 @@ class BrowserToolbar @JvmOverloads constructor( } /** - * Set/Get the site security icon colours (usually a lock or globe icon). It uses a pair of integers + * 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. */ - var siteSecurityColor: Pair - get() = displayToolbar.securityIconColor - set(value) { displayToolbar.securityIconColor = value } + var siteSecurityIcons + get() = displayToolbar.securityIcons + set(value) { displayToolbar.securityIcons = value } /** * Gets/Sets a custom view that will be drawn as behind the URL and page actions in display mode. @@ -468,15 +469,11 @@ class BrowserToolbar @JvmOverloads constructor( R.styleable.BrowserToolbar_browserToolbarSuggestionBackgroundColor, suggestionBackgroundColor ) - val inSecure = getColor( - R.styleable.BrowserToolbar_browserToolbarInsecureColor, - displayToolbar.securityIconColor.first - ) - val secure = getColor( - R.styleable.BrowserToolbar_browserToolbarSecureColor, - displayToolbar.securityIconColor.second - ) - siteSecurityColor = Pair(inSecure, secure) + val inSecure = getDrawable(R.styleable.BrowserToolbar_browserToolbarInsecureIcon) + ?: displayToolbar.securityIcons.insecure + val secure = getDrawable(R.styleable.BrowserToolbar_browserToolbarSecureIcon) + ?: displayToolbar.securityIcons.secure + siteSecurityIcons = SiteSecurityIcons(inSecure, secure) val fadingEdgeLength = getDimensionPixelSize( R.styleable.BrowserToolbar_browserToolbarFadingEdgeSize, resources.getDimensionPixelSize(R.dimen.mozac_browser_toolbar_url_fading_edge_size) diff --git a/components/browser/toolbar/src/main/java/mozilla/components/browser/toolbar/display/DisplayToolbar.kt b/components/browser/toolbar/src/main/java/mozilla/components/browser/toolbar/display/DisplayToolbar.kt index 8fc1c1cb85e..272cbb38972 100644 --- a/components/browser/toolbar/src/main/java/mozilla/components/browser/toolbar/display/DisplayToolbar.kt +++ b/components/browser/toolbar/src/main/java/mozilla/components/browser/toolbar/display/DisplayToolbar.kt @@ -33,8 +33,6 @@ import mozilla.components.concept.toolbar.Toolbar.SiteTrackingProtection.ON_NO_T 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 -import mozilla.components.ui.icons.R.drawable.mozac_ic_globe -import mozilla.components.ui.icons.R.drawable.mozac_ic_lock /** * Sub-component of the browser toolbar responsible for displaying the URL and related controls. @@ -89,6 +87,58 @@ internal class DisplayToolbar( setTrackingProtectionState(siteTrackingProtection) } + private val browserActions: MutableList = mutableListOf() + private val pageActions: MutableList = mutableListOf() + private val navigationActions: MutableList = mutableListOf() + + // Margin between browser actions. + internal var browserActionMargin = 0 + + // Location of progress bar + internal var progressBarGravity = BOTTOM_PROGRESS_BAR + + // Horizontal margin of URL Box (surrounding URL and page actions). + internal var urlBoxMargin = 0 + + // An optional view to be drawn behind the URL and page actions. + internal var urlBoxView: View? = null + set(value) { + // Remove previous view from ViewGroup + if (field != null) { + removeView(field) + } + // Add new view to ViewGroup (at position 0 to be drawn *before* the URL and page actions) + if (value != null) { + addView(value, 0) + } + field = value + } + + // Callback to determine whether to open edit mode or not. + internal var onUrlClicked: () -> Boolean = { true } + + private val defaultColor = ContextCompat.getColor(context, R.color.photonWhite) + + @ColorInt + internal var separatorColor = + ContextCompat.getColor(context, R.color.photonGrey80) + + private var currentSiteSecurity = SiteSecurity.INSECURE + + private var siteTrackingProtection = OFF_GLOBALLY + + internal var securityIcons = SiteSecurityIcons.getDefaultSecurityIcons(context, defaultColor) + set(value) { + field = value + setSiteSecurity(currentSiteSecurity) + } + + internal var menuViewColor = defaultColor + set(value) { + field = value + menuView.setColorFilter(value) + } + internal val trackingProtectionIconView = TrackingProtectionIconView(context).apply { isVisible = false setImageResource(R.drawable.mozac_tracking_protection_state_list) @@ -113,7 +163,7 @@ internal class DisplayToolbar( internal val siteSecurityIconView = AppCompatImageView(context).apply { setPadding(resources.getDimensionPixelSize(R.dimen.mozac_browser_toolbar_icon_padding)) - setImageResource(mozac_ic_globe) + 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. @@ -165,58 +215,6 @@ internal class DisplayToolbar( }) } - private val browserActions: MutableList = mutableListOf() - private val pageActions: MutableList = mutableListOf() - private val navigationActions: MutableList = mutableListOf() - - // Margin between browser actions. - internal var browserActionMargin = 0 - - // Location of progress bar - internal var progressBarGravity = BOTTOM_PROGRESS_BAR - - // Horizontal margin of URL Box (surrounding URL and page actions). - internal var urlBoxMargin = 0 - - // An optional view to be drawn behind the URL and page actions. - internal var urlBoxView: View? = null - set(value) { - // Remove previous view from ViewGroup - if (field != null) { - removeView(field) - } - // Add new view to ViewGroup (at position 0 to be drawn *before* the URL and page actions) - if (value != null) { - addView(value, 0) - } - field = value - } - - // Callback to determine whether to open edit mode or not. - internal var onUrlClicked: () -> Boolean = { true } - - private val defaultColor = ContextCompat.getColor(context, R.color.photonWhite) - - @ColorInt - internal var separatorColor = - ContextCompat.getColor(context, R.color.photonGrey80) - - private var currentSiteSecurity = SiteSecurity.INSECURE - - private var siteTrackingProtection = OFF_GLOBALLY - - internal var securityIconColor = Pair(defaultColor, defaultColor) - set(value) { - field = value - setSiteSecurity(currentSiteSecurity) - } - - internal var menuViewColor = defaultColor - set(value) { - field = value - menuView.setColorFilter(value) - } - init { addView(trackingProtectionIconView) addView(trackingProtectionAndSecurityIndicatorSeparatorView) @@ -307,14 +305,11 @@ internal class DisplayToolbar( * Sets the site's security icon as secure if true, else the regular globe. */ fun setSiteSecurity(secure: SiteSecurity) { - val (image, color) = when (secure) { - SiteSecurity.INSECURE -> Pair(mozac_ic_globe, securityIconColor.first) - SiteSecurity.SECURE -> Pair(mozac_ic_lock, securityIconColor.second) - } - siteSecurityIconView.apply { - setImageResource(image) - setColorFilter(color) + val drawable = when (secure) { + SiteSecurity.INSECURE -> securityIcons.insecure + SiteSecurity.SECURE -> securityIcons.secure } + siteSecurityIconView.setImageDrawable(drawable) currentSiteSecurity = secure } diff --git a/components/browser/toolbar/src/main/java/mozilla/components/browser/toolbar/display/SiteSecurityIcons.kt b/components/browser/toolbar/src/main/java/mozilla/components/browser/toolbar/display/SiteSecurityIcons.kt new file mode 100644 index 00000000000..171b3b46c47 --- /dev/null +++ b/components/browser/toolbar/src/main/java/mozilla/components/browser/toolbar/display/SiteSecurityIcons.kt @@ -0,0 +1,61 @@ +/* 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.graphics.BlendMode +import android.graphics.BlendModeColorFilter +import android.graphics.ColorFilter +import android.graphics.PorterDuff +import android.graphics.PorterDuffColorFilter +import android.graphics.drawable.Drawable +import android.os.Build +import android.os.Build.VERSION.SDK_INT +import androidx.annotation.ColorInt +import mozilla.components.ui.icons.R + +/** + * Specifies icons to display in the toolbar representing the security of the current website. + * + * @property insecure Icon to display for HTTP sites. + * @property secure Icon to display for HTTPS sites. + */ +data class SiteSecurityIcons( + val insecure: Drawable?, + val secure: Drawable? +) { + + /** + * Returns an instance of [SiteSecurityIcons] with a color filter applied to both icons. + */ + fun withColorFilter(newColorFilter: ColorFilter): SiteSecurityIcons { + return copy( + insecure = insecure?.apply { colorFilter = newColorFilter }, + secure = secure?.apply { colorFilter = newColorFilter } + ) + } + + /** + * Returns an instance of [SiteSecurityIcons] with a color tint applied to both icons. + */ + fun withColorFilter(@ColorInt newColor: Int): SiteSecurityIcons { + val newColorFilter: ColorFilter = if (SDK_INT >= Build.VERSION_CODES.Q) { + BlendModeColorFilter(newColor, BlendMode.SRC_IN) + } else { + PorterDuffColorFilter(newColor, PorterDuff.Mode.SRC_IN) + } + + return withColorFilter(newColorFilter) + } + + companion object { + fun getDefaultSecurityIcons(context: Context, @ColorInt color: Int): SiteSecurityIcons { + return SiteSecurityIcons( + insecure = context.getDrawable(R.drawable.mozac_ic_globe), + secure = context.getDrawable(R.drawable.mozac_ic_lock) + ).withColorFilter(color) + } + } +} 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 index 38754d196c5..106bec1251e 100644 --- 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,8 +8,8 @@ - - + + diff --git a/components/browser/toolbar/src/test/java/mozilla/components/browser/toolbar/display/DisplayToolbarTest.kt b/components/browser/toolbar/src/test/java/mozilla/components/browser/toolbar/display/DisplayToolbarTest.kt index d574395ae06..abc4939c653 100644 --- a/components/browser/toolbar/src/test/java/mozilla/components/browser/toolbar/display/DisplayToolbarTest.kt +++ b/components/browser/toolbar/src/test/java/mozilla/components/browser/toolbar/display/DisplayToolbarTest.kt @@ -47,6 +47,14 @@ import org.robolectric.Shadows.shadowOf @RunWith(AndroidJUnit4::class) class DisplayToolbarTest { + @Test + fun `initialized with security icon`() { + val toolbar = mock(BrowserToolbar::class.java) + val displayToolbar = DisplayToolbar(testContext, toolbar) + + assertNotNull(displayToolbar.siteSecurityIconView.drawable) + } + @Test fun `clicking on the URL switches the toolbar to editing mode`() { val toolbar = mock(BrowserToolbar::class.java) @@ -970,26 +978,27 @@ class DisplayToolbarTest { } @Test - fun `securityIconColor is set when securityIconColor changes`() { + fun `securityIcons is set when securityIcons changes`() { val toolbar = mock(BrowserToolbar::class.java) val displayToolbar = DisplayToolbar(testContext, toolbar) - displayToolbar.securityIconColor = Pair(R.color.photonBlue40, R.color.photonBlue40) + val insecure = testContext.getDrawable(R.drawable.mozac_ic_globe) + val secure = testContext.getDrawable(R.drawable.mozac_ic_lock) + displayToolbar.securityIcons = SiteSecurityIcons(insecure, secure) - assertEquals(R.color.photonBlue40, displayToolbar.securityIconColor.first) - assertEquals(R.color.photonBlue40, displayToolbar.securityIconColor.second) + assertEquals(insecure, displayToolbar.securityIcons.insecure) + assertEquals(secure, displayToolbar.securityIcons.secure) } @Test - fun `setSiteSecurity is called when securityIconColor changes`() { + fun `setSiteSecurity is called when securityIcons changes`() { val toolbar = BrowserToolbar(testContext) - toolbar.displayToolbar - - assertNull(toolbar.displayToolbar.siteSecurityIconView.colorFilter) - toolbar.siteSecurityColor = Pair(R.color.photonBlue40, R.color.photonBlue40) + val icons = SiteSecurityIcons(mock(), mock()) + assertNotEquals(icons, toolbar.displayToolbar.securityIcons) - assertNotNull(toolbar.displayToolbar.siteSecurityIconView.colorFilter) + toolbar.siteSecurityIcons = icons + assertEquals(icons, toolbar.displayToolbar.securityIcons) } @Test diff --git a/components/browser/toolbar/src/test/java/mozilla/components/browser/toolbar/display/MenuButtonTest.kt b/components/browser/toolbar/src/test/java/mozilla/components/browser/toolbar/display/MenuButtonTest.kt index 53753fe0a3a..645e4e0780f 100644 --- a/components/browser/toolbar/src/test/java/mozilla/components/browser/toolbar/display/MenuButtonTest.kt +++ b/components/browser/toolbar/src/test/java/mozilla/components/browser/toolbar/display/MenuButtonTest.kt @@ -14,6 +14,7 @@ import androidx.annotation.ColorInt import androidx.annotation.ColorRes import androidx.appcompat.widget.AppCompatImageView import androidx.core.view.iterator +import androidx.test.ext.junit.runners.AndroidJUnit4 import mozilla.components.browser.menu.BrowserMenu import mozilla.components.browser.menu.BrowserMenuBuilder import mozilla.components.browser.menu.item.BrowserMenuHighlightableItem @@ -29,27 +30,26 @@ import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.mockito.ArgumentMatchers.anyBoolean +import org.mockito.Mock import org.mockito.Mockito.`when` import org.mockito.Mockito.doReturn import org.mockito.Mockito.never import org.mockito.Mockito.spy import org.mockito.Mockito.verify -import org.robolectric.RobolectricTestRunner +import org.mockito.MockitoAnnotations.initMocks -@RunWith(RobolectricTestRunner::class) +@RunWith(AndroidJUnit4::class) class MenuButtonTest { - private lateinit var parent: View + @Mock private lateinit var parent: View + @Mock private lateinit var menuBuilder: BrowserMenuBuilder + @Mock private lateinit var menu: BrowserMenu private lateinit var menuButton: MenuButton - private lateinit var menuBuilder: BrowserMenuBuilder - private lateinit var menu: BrowserMenu @Before fun setup() { - parent = mock() + initMocks(this) menuButton = MenuButton(testContext, parent) - menuBuilder = mock() - menu = mock() `when`(parent.layoutParams).thenReturn(mock()) `when`(menuBuilder.build(testContext)).thenReturn(menu) @@ -129,7 +129,6 @@ class MenuButtonTest { private fun extractIcon() = menuButton.iterator().asSequence().find { it is AppCompatImageView } as AppCompatImageView - @Suppress("DEPRECATION") private fun mockContextWithColorResource(@ColorRes resId: Int, @ColorInt color: Int): Context { val context: Context = spy(testContext) @@ -141,7 +140,9 @@ class MenuButtonTest { doReturn(res).`when`(context).resources doReturn(color).`when`(context).getColor(resId) + @Suppress("Deprecation") doReturn(color).`when`(res).getColor(resId) + return context } } diff --git a/components/browser/toolbar/src/test/java/mozilla/components/browser/toolbar/display/SiteSecurityIconsTest.kt b/components/browser/toolbar/src/test/java/mozilla/components/browser/toolbar/display/SiteSecurityIconsTest.kt new file mode 100644 index 00000000000..488b1b90285 --- /dev/null +++ b/components/browser/toolbar/src/test/java/mozilla/components/browser/toolbar/display/SiteSecurityIconsTest.kt @@ -0,0 +1,55 @@ +/* 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.graphics.Color +import android.graphics.ColorMatrix +import android.graphics.ColorMatrixColorFilter +import android.graphics.PorterDuff +import android.graphics.PorterDuffColorFilter +import android.graphics.drawable.Drawable +import androidx.test.ext.junit.runners.AndroidJUnit4 +import mozilla.components.support.test.mock +import mozilla.components.support.test.robolectric.testContext +import org.junit.Assert.assertEquals +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mockito.verify + +@RunWith(AndroidJUnit4::class) +class SiteSecurityIconsTest { + + @Test + fun `default returns non-null tinted icons`() { + val icons = SiteSecurityIcons.getDefaultSecurityIcons(testContext, Color.RED) + assertEquals(PorterDuffColorFilter(Color.RED, PorterDuff.Mode.SRC_IN), icons.insecure?.colorFilter) + assertEquals(PorterDuffColorFilter(Color.RED, PorterDuff.Mode.SRC_IN), icons.secure?.colorFilter) + } + + @Test + fun `withColorFilter tints existing drawables`() { + val insecure: Drawable = mock() + val secure: Drawable = mock() + val icons = SiteSecurityIcons(insecure, secure).withColorFilter(Color.BLUE) + + assertEquals(insecure, icons.insecure) + assertEquals(secure, icons.secure) + verify(insecure).colorFilter = PorterDuffColorFilter(Color.BLUE, PorterDuff.Mode.SRC_IN) + verify(secure).colorFilter = PorterDuffColorFilter(Color.BLUE, PorterDuff.Mode.SRC_IN) + } + + @Test + fun `withColorFilter allows custom filters to be used`() { + val insecure: Drawable = mock() + val secure: Drawable = mock() + val filter = ColorMatrixColorFilter(ColorMatrix()) + val icons = SiteSecurityIcons(insecure, secure).withColorFilter(filter) + + assertEquals(insecure, icons.insecure) + assertEquals(secure, icons.secure) + verify(insecure).colorFilter = filter + verify(secure).colorFilter = filter + } +} diff --git a/components/feature/customtabs/src/main/java/mozilla/components/feature/customtabs/CustomTabsToolbarFeature.kt b/components/feature/customtabs/src/main/java/mozilla/components/feature/customtabs/CustomTabsToolbarFeature.kt index eb88717bd87..7e2806aff24 100644 --- a/components/feature/customtabs/src/main/java/mozilla/components/feature/customtabs/CustomTabsToolbarFeature.kt +++ b/components/feature/customtabs/src/main/java/mozilla/components/feature/customtabs/CustomTabsToolbarFeature.kt @@ -101,7 +101,7 @@ class CustomTabsToolbarFeature( toolbar.setBackgroundColor(color) toolbar.textColor = readableColor toolbar.titleColor = readableColor - toolbar.siteSecurityColor = Pair(readableColor, readableColor) + toolbar.siteSecurityIcons = toolbar.siteSecurityIcons.withColorFilter(readableColor) toolbar.menuViewColor = readableColor window?.setStatusBarTheme(color) diff --git a/docs/changelog.md b/docs/changelog.md index f529cc7d4ae..0c3272164ab 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -12,6 +12,8 @@ permalink: /changelog/ * [Gecko](https://github.com/mozilla-mobile/android-components/blob/master/buildSrc/src/main/java/Gecko.kt) * [Configuration](https://github.com/mozilla-mobile/android-components/blob/master/buildSrc/src/main/java/Config.kt) +* **browser-toolbar** + * ⚠️ **This is a breaking change**: The new `BrowserToolbar.siteSecurityIcons` replaces `BrowserToolbar.siteSecurityColor` and lets custom security icons with multiple colors be used in the toolbar. # 7.0.0 @@ -37,7 +39,7 @@ permalink: /changelog/ // passed-in context allows easily opening new activities for handling urls. tabsUseCases.addTab(authUrl) } - + // ... elsewhere, in the UI code, handling click on button "Sign In": components.feature.beginAuthentication(activityContext) ``` @@ -68,6 +70,9 @@ permalink: /changelog/ * **service-firefox-accounts** * ⚠️ **This is a breaking change**: `AccountObserver.onAuthenticated` now helps observers distinguish when an account is a new authenticated account one with a second `newAccount` boolean parameter. +* **feature-session** + * ⚠️ **This is a breaking change**: The new `BrowserToolbar.siteSecurityIcons` replaces `BrowserToolbar.siteSecurityColor` and lets custom security icons with multiple colors be used in the toolbar. + # 6.0.2 * [Commits](https://github.com/mozilla-mobile/android-components/compare/v6.0.1...v6.0.2)