Commit c860e740 authored by Tomasz Wiszkowski's avatar Tomasz Wiszkowski Committed by Commit Bot

Use dark theme controlled palette for UrlBar text colors.

This change:
- reduces code complexity and dependency on duplicate resources,
- fixes a glitch on Android P and Q where text colors would not be
  correctly reflected by the theme,
- drops redundant color definitions.

Bug: 944496

Change-Id: I3058edc9c6414ead398bb4e7f3d223fd0a8c77d8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1536541Reviewed-by: default avatarYusuf Ozuysal <yusufo@chromium.org>
Reviewed-by: default avatarBecky Zhou <huayinz@chromium.org>
Commit-Queue: Ender <ender@google.com>
Cr-Commit-Position: refs/heads/master@{#644378}
parent e7bed62b
...@@ -73,9 +73,6 @@ ...@@ -73,9 +73,6 @@
<color name="url_emphasis_light_non_emphasized_text">@color/white_alpha_50</color> <color name="url_emphasis_light_non_emphasized_text">@color/white_alpha_50</color>
<color name="url_emphasis_domain_and_registry">#333333</color> <color name="url_emphasis_domain_and_registry">#333333</color>
<color name="url_emphasis_light_domain_and_registry">@android:color/white</color> <color name="url_emphasis_light_domain_and_registry">@android:color/white</color>
<color name="url_emphasis_default_text">@color/modern_grey_900</color>
<!--suppress UnusedResources -->
<color name="url_emphasis_light_default_text">@android:color/white</color>
<!-- Omnibox Suggestion colors --> <!-- Omnibox Suggestion colors -->
<color name="suggestion_url_dark_modern">@color/default_text_color_link</color> <color name="suggestion_url_dark_modern">@color/default_text_color_link</color>
......
...@@ -51,7 +51,6 @@ import org.chromium.chrome.browser.preferences.privacy.PrivacyPreferencesManager ...@@ -51,7 +51,6 @@ import org.chromium.chrome.browser.preferences.privacy.PrivacyPreferencesManager
import org.chromium.chrome.browser.profiles.Profile; import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.search_engines.TemplateUrlService; import org.chromium.chrome.browser.search_engines.TemplateUrlService;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.toolbar.LocationBarModel;
import org.chromium.chrome.browser.toolbar.ToolbarDataProvider; import org.chromium.chrome.browser.toolbar.ToolbarDataProvider;
import org.chromium.chrome.browser.toolbar.ToolbarManager; import org.chromium.chrome.browser.toolbar.ToolbarManager;
import org.chromium.chrome.browser.toolbar.top.ToolbarActionModeCallback; import org.chromium.chrome.browser.toolbar.top.ToolbarActionModeCallback;
...@@ -105,8 +104,7 @@ public class LocationBarLayout extends FrameLayout ...@@ -105,8 +104,7 @@ public class LocationBarLayout extends FrameLayout
private OmniboxPrerender mOmniboxPrerender; private OmniboxPrerender mOmniboxPrerender;
private boolean mUseDarkColors; private boolean mOmniboxVoiceSearchAlwaysVisible;
protected float mUrlFocusChangePercent; protected float mUrlFocusChangePercent;
protected LinearLayout mUrlActionContainer; protected LinearLayout mUrlActionContainer;
...@@ -1024,22 +1022,21 @@ public class LocationBarLayout extends FrameLayout ...@@ -1024,22 +1022,21 @@ public class LocationBarLayout extends FrameLayout
ColorUtils.getDefaultThemeColor(getResources(), mToolbarDataProvider.isIncognito()); ColorUtils.getDefaultThemeColor(getResources(), mToolbarDataProvider.isIncognito());
final int primaryColor = final int primaryColor =
mUrlHasFocus ? defaultPrimaryColor : mToolbarDataProvider.getPrimaryColor(); mUrlHasFocus ? defaultPrimaryColor : mToolbarDataProvider.getPrimaryColor();
mUseDarkColors = final boolean useDarkColors = !ColorUtils.shouldUseLightForegroundOnBackground(primaryColor);
LocationBarModel.shouldUseDarkColors(mToolbarDataProvider.hasTab(), primaryColor);
int id = ColorUtils.getIconTintRes(!mUseDarkColors); int id = ColorUtils.getIconTintRes(!useDarkColors);
ColorStateList colorStateList = AppCompatResources.getColorStateList(getContext(), id); ColorStateList colorStateList = AppCompatResources.getColorStateList(getContext(), id);
ApiCompatibilityUtils.setImageTintList(mMicButton, colorStateList); ApiCompatibilityUtils.setImageTintList(mMicButton, colorStateList);
ApiCompatibilityUtils.setImageTintList(mDeleteButton, colorStateList); ApiCompatibilityUtils.setImageTintList(mDeleteButton, colorStateList);
// If the URL changed colors and is not focused, update the URL to account for the new // If the URL changed colors and is not focused, update the URL to account for the new
// color scheme. // color scheme.
if (mUrlCoordinator.setUseDarkTextColors(mUseDarkColors) && !mUrlBar.hasFocus()) { if (mUrlCoordinator.setUseDarkTextColors(useDarkColors) && !mUrlBar.hasFocus()) {
setUrlToPageUrl(); setUrlToPageUrl();
} }
mStatusViewCoordinator.setUseDarkColors(mUseDarkColors); mStatusViewCoordinator.setUseDarkColors(useDarkColors);
mAutocompleteCoordinator.updateVisualsForState(mUseDarkColors); mAutocompleteCoordinator.updateVisualsForState(useDarkColors);
} }
@Override @Override
......
...@@ -183,8 +183,8 @@ public class OmniboxUrlEmphasizer { ...@@ -183,8 +183,8 @@ public class OmniboxUrlEmphasizer {
// Draw attention to the data: URI scheme for anti-spoofing reasons. // Draw attention to the data: URI scheme for anti-spoofing reasons.
if (UrlConstants.DATA_SCHEME.equals( if (UrlConstants.DATA_SCHEME.equals(
emphasizeResponse.extractScheme(urlString))) { emphasizeResponse.extractScheme(urlString))) {
colorId = useDarkColors ? R.color.url_emphasis_default_text colorId = useDarkColors ? R.color.default_text_color_dark
: R.color.url_emphasis_light_default_text; : R.color.default_text_color_light;
} }
break; break;
case ConnectionSecurityLevel.DANGEROUS: case ConnectionSecurityLevel.DANGEROUS:
......
...@@ -91,14 +91,12 @@ class UrlBarViewBinder { ...@@ -91,14 +91,12 @@ class UrlBarViewBinder {
int hintColor; int hintColor;
int highlightColor; int highlightColor;
if (useDarkTextColors) { if (useDarkTextColors) {
textColor = textColor = ApiCompatibilityUtils.getColor(resources, R.color.default_text_color_dark);
ApiCompatibilityUtils.getColor(resources, R.color.url_emphasis_default_text);
hintColor = hintColor =
ApiCompatibilityUtils.getColor(resources, R.color.locationbar_dark_hint_text); ApiCompatibilityUtils.getColor(resources, R.color.locationbar_dark_hint_text);
highlightColor = originalHighlightColor; highlightColor = originalHighlightColor;
} else { } else {
textColor = ApiCompatibilityUtils.getColor( textColor = ApiCompatibilityUtils.getColor(resources, R.color.default_text_color_light);
resources, R.color.url_emphasis_light_default_text);
hintColor = hintColor =
ApiCompatibilityUtils.getColor(resources, R.color.locationbar_light_hint_text); ApiCompatibilityUtils.getColor(resources, R.color.locationbar_light_hint_text);
highlightColor = ApiCompatibilityUtils.getColor( highlightColor = ApiCompatibilityUtils.getColor(
......
...@@ -131,8 +131,10 @@ public class BasicSuggestionProcessor implements SuggestionProcessor { ...@@ -131,8 +131,10 @@ public class BasicSuggestionProcessor implements SuggestionProcessor {
if ((suggestionType == OmniboxSuggestionType.SEARCH_SUGGEST_ENTITY) if ((suggestionType == OmniboxSuggestionType.SEARCH_SUGGEST_ENTITY)
|| (suggestionType == OmniboxSuggestionType.SEARCH_SUGGEST_PROFILE)) { || (suggestionType == OmniboxSuggestionType.SEARCH_SUGGEST_PROFILE)) {
textLine2 = SpannableString.valueOf(suggestion.getDescription()); textLine2 = SpannableString.valueOf(suggestion.getDescription());
textLine2Color = SuggestionViewViewBinder.getStandardFontColor( textLine2Color = ApiCompatibilityUtils.getColor(mContext.getResources(),
mContext, model.get(SuggestionCommonProperties.USE_DARK_COLORS)); model.get(SuggestionCommonProperties.USE_DARK_COLORS)
? R.color.default_text_color_dark
: R.color.default_text_color_light);
textLine2Direction = View.TEXT_DIRECTION_INHERIT; textLine2Direction = View.TEXT_DIRECTION_INHERIT;
} else if (mEnableNewAnswerLayout } else if (mEnableNewAnswerLayout
&& suggestionType == OmniboxSuggestionType.CALCULATOR) { && suggestionType == OmniboxSuggestionType.CALCULATOR) {
......
...@@ -4,9 +4,6 @@ ...@@ -4,9 +4,6 @@
package org.chromium.chrome.browser.omnibox.suggestions.basic; package org.chromium.chrome.browser.omnibox.suggestions.basic;
import android.content.Context;
import android.support.annotation.ColorInt;
import android.support.annotation.ColorRes;
import android.support.v4.view.ViewCompat; import android.support.v4.view.ViewCompat;
import android.text.Spannable; import android.text.Spannable;
import android.text.TextUtils; import android.text.TextUtils;
...@@ -37,7 +34,9 @@ public class SuggestionViewViewBinder { ...@@ -37,7 +34,9 @@ public class SuggestionViewViewBinder {
view.updateRefineIconTint(useDarkColors); view.updateRefineIconTint(useDarkColors);
view.updateSuggestionIconTint(useDarkColors); view.updateSuggestionIconTint(useDarkColors);
view.getTextLine1().setTextColor( view.getTextLine1().setTextColor(
getStandardFontColor(view.getContext(), useDarkColors)); ApiCompatibilityUtils.getColor(view.getContext().getResources(),
useDarkColors ? R.color.default_text_color_dark
: R.color.default_text_color_light));
} else if (SuggestionCommonProperties.LAYOUT_DIRECTION.equals(propertyKey)) { } else if (SuggestionCommonProperties.LAYOUT_DIRECTION.equals(propertyKey)) {
ViewCompat.setLayoutDirection( ViewCompat.setLayoutDirection(
view, model.get(SuggestionCommonProperties.LAYOUT_DIRECTION)); view, model.get(SuggestionCommonProperties.LAYOUT_DIRECTION));
...@@ -157,18 +156,4 @@ public class SuggestionViewViewBinder { ...@@ -157,18 +156,4 @@ public class SuggestionViewViewBinder {
: SuggestionView.SuggestionLayoutType.ANSWER); : SuggestionView.SuggestionLayoutType.ANSWER);
} }
} }
/**
* Get the appropriate font color to be used for non-URL text in suggestions.
* @param context The context to load the color.
* @param useDarkColors Whether dark colors should be used.
* @return The font color to be used.
*/
@ColorInt
public static int getStandardFontColor(Context context, boolean useDarkColors) {
@ColorRes
int res = useDarkColors ? R.color.url_emphasis_default_text
: R.color.url_emphasis_light_default_text;
return ApiCompatibilityUtils.getColor(context.getResources(), res);
}
} }
...@@ -5,7 +5,6 @@ ...@@ -5,7 +5,6 @@
package org.chromium.chrome.browser.toolbar; package org.chromium.chrome.browser.toolbar;
import android.content.Context; import android.content.Context;
import android.support.annotation.ColorInt;
import android.support.annotation.ColorRes; import android.support.annotation.ColorRes;
import android.support.annotation.DrawableRes; import android.support.annotation.DrawableRes;
import android.support.annotation.Nullable; import android.support.annotation.Nullable;
...@@ -225,7 +224,8 @@ public class LocationBarModel implements ToolbarDataProvider { ...@@ -225,7 +224,8 @@ public class LocationBarModel implements ToolbarDataProvider {
OmniboxUrlEmphasizer.emphasizeUrl(spannableDisplayText, mContext.getResources(), OmniboxUrlEmphasizer.emphasizeUrl(spannableDisplayText, mContext.getResources(),
getProfile(), getSecurityLevel(), isInternalPage, getProfile(), getSecurityLevel(), isInternalPage,
shouldUseDarkColors(hasTab(), getPrimaryColor()), shouldEmphasizeHttpsScheme()); !ColorUtils.shouldUseLightForegroundOnBackground(getPrimaryColor()),
shouldEmphasizeHttpsScheme());
} }
return UrlBarData.forUrlAndText(url, spannableDisplayText, editingText); return UrlBarData.forUrlAndText(url, spannableDisplayText, editingText);
...@@ -250,16 +250,6 @@ public class LocationBarModel implements ToolbarDataProvider { ...@@ -250,16 +250,6 @@ public class LocationBarModel implements ToolbarDataProvider {
return !isUsingBrandColor() && !isIncognito(); return !isUsingBrandColor() && !isIncognito();
} }
/**
* @param hasTab Whether the location is attached to a {@link Tab}. See
* {@link ToolbarDataProvider#hasTab()}.
* @param primaryColor The primary color of the toolbar that holds this location bar.
* @return Whether or not dark text color or icon tint should be used for the location bar.
*/
public static boolean shouldUseDarkColors(boolean hasTab, @ColorInt int primaryColor) {
return !hasTab || !ColorUtils.shouldUseLightForegroundOnBackground(primaryColor);
}
@Override @Override
public String getTitle() { public String getTitle() {
if (!hasTab()) return ""; if (!hasTab()) return "";
......
...@@ -496,11 +496,9 @@ public class CustomTabToolbar ...@@ -496,11 +496,9 @@ public class CustomTabToolbar
setUrlToPageUrl(); setUrlToPageUrl();
} }
int titleTextColor = mUseDarkColors mTitleBar.setTextColor(ApiCompatibilityUtils.getColor(resources,
? ApiCompatibilityUtils.getColor(resources, R.color.url_emphasis_default_text) mUseDarkColors ? R.color.default_text_color_dark
: ApiCompatibilityUtils.getColor( : R.color.default_text_color_light));
resources, R.color.url_emphasis_light_default_text);
mTitleBar.setTextColor(titleTextColor);
if (getProgressBar() != null) { if (getProgressBar() != null) {
if (!ColorUtils.isUsingDefaultToolbarColor( if (!ColorUtils.isUsingDefaultToolbarColor(
......
...@@ -294,7 +294,7 @@ public class OmniboxUrlEmphasizerTest { ...@@ -294,7 +294,7 @@ public class OmniboxUrlEmphasizerTest {
Assert.assertEquals("Unexpected number of spans:", 2, spans.length); Assert.assertEquals("Unexpected number of spans:", 2, spans.length);
spans[0].assertIsColoredSpan("data", 0, spans[0].assertIsColoredSpan("data", 0,
ApiCompatibilityUtils.getColor(mResources, R.color.url_emphasis_default_text)); ApiCompatibilityUtils.getColor(mResources, R.color.default_text_color_dark));
spans[1].assertIsColoredSpan(":text/plain;charset=utf-8;base64,VGVzdCBVUkw=", 4, spans[1].assertIsColoredSpan(":text/plain;charset=utf-8;base64,VGVzdCBVUkw=", 4,
ApiCompatibilityUtils.getColor( ApiCompatibilityUtils.getColor(
mResources, R.color.url_emphasis_non_emphasized_text)); mResources, R.color.url_emphasis_non_emphasized_text));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment