Skip to content

GitLab

T
tangled
Commit 02aa4dda authored by Haiyang Pan's avatar Haiyang Pan Committed by Commit Bot
Browse files
Options

Revert "[Snowflake] Prevent from adding hex color values outside color_palette.xml" 

This reverts commit 13908ddd.

Reason for revert: It prevents uploading a new patchset for an internal CL which does not change any xml file

Original change's description:
> [Snowflake] Prevent from adding hex color values outside color_palette.xml
>
> Moved some semantic names in color_palette.xml to semantic_colors.xml.
> And added some presubmit checks to prevent from adding more hex argb
> values in color resource files except color_palette.xml.
>
> Bug: 775198
> Change-Id: Ie27dd8dc0e4fa6506ed192c6f5f9c4b5b5f88566
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1896241
> Commit-Queue: Lijin Shen <lazzzis@google.com>
> Reviewed-by: Theresa  <twellington@chromium.org>
> Reviewed-by: John Budorick <jbudorick@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#712462}

TBR=twellington@chromium.org,jbudorick@chromium.org,lazzzis@google.com

Change-Id: Ib41c469abd121a3003bdd1b7047ff6dac9f0b93f
Bug: 775198
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1900657Reviewed-by: default avatarJohn Budorick <jbudorick@chromium.org>
Commit-Queue: John Budorick <jbudorick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712881}
parent 06420557
Show whitespace changes
Inline Side-by-side
Showing with 91 additions and 192 deletions
tools/android/checkxmlstyle/checkxmlstyle.py
View file @ 02aa4dda
......@@ -6,9 +6,8 @@
This file checks for the following:
- Colors are defined as RRGGBB or AARRGGBB
- No (A)RGB values are referenced outside color_palette.xml
- No duplicate (A)RGB values are referenced in color_palette.xml
- Colors in semantic_colors are only referecing colors in color_palette.xml
- No (A)RGB values are referenced outside colors.xml
- No duplicate (A)RGB values are referenced in colors.xml
- XML namspace "app" is used for "http://schemas.android.com/apk/res-auto"
- Android text attributes are only defined in text appearance styles
- Warning on adding new text appearance styles
......@@ -19,7 +18,16 @@ import os
import re
import xml.etree.ElementTree as ET
import helpers
COLOR_PATTERN = re.compile(r'(>|")(#[0-9A-Fa-f]+)(<|")')
VALID_COLOR_PATTERN = re.compile(
r'^#([0-9A-F][0-9A-E]|[0-9A-E][0-9A-F])?[0-9A-F]{6}$')
XML_APP_NAMESPACE_PATTERN = re.compile(
r'xmlns:(\w+)="http://schemas.android.com/apk/res-auto"')
TEXT_APPEARANCE_STYLE_PATTERN = re.compile(r'^TextAppearance\.')
INCLUDED_PATHS = [
r'^(chrome|ui|components|content)[\\/](.*[\\/])?java[\\/]res.+\.xml$'
]
def CheckStyleOnUpload(input_api, output_api):
"""Returns result for all the presubmit upload checks for XML files."""
......@@ -35,8 +43,7 @@ def CheckStyleOnCommit(input_api, output_api):
def IncludedFiles(input_api):
# Filter out XML files outside included paths and files that were deleted.
files = lambda f: input_api.FilterSourceFile(
f, white_list=helpers.INCLUDED_PATHS)
files = lambda f: input_api.FilterSourceFile(f, white_list=INCLUDED_PATHS)
return input_api.AffectedFiles(include_deletes=False, file_filter=files)
......@@ -46,13 +53,12 @@ def _CommonChecks(input_api, output_api):
result.extend(_CheckColorFormat(input_api, output_api))
result.extend(_CheckColorReferences(input_api, output_api))
result.extend(_CheckDuplicateColors(input_api, output_api))
result.extend(_CheckSemanticColorsReferences(input_api, output_api))
result.extend(_CheckXmlNamespacePrefixes(input_api, output_api))
result.extend(_CheckTextAppearance(input_api, output_api))
# Add more checks here
return result
### color resources below ###
def _CheckColorFormat(input_api, output_api):
"""Checks color (A)RGB values are of format either RRGGBB or AARRGGBB."""
errors = []
......@@ -62,8 +68,8 @@ def _CheckColorFormat(input_api, output_api):
if '<vector' in contents:
continue
for line_number, line in f.ChangedContents():
color = helpers.COLOR_PATTERN.search(line)
if color and not helpers.VALID_COLOR_PATTERN.match(color.group(2)):
color = COLOR_PATTERN.search(line)
if color and not VALID_COLOR_PATTERN.match(color.group(2)):
errors.append(
' %s:%d\n \t%s' % (f.LocalPath(), line_number, line.strip()))
if errors:
......@@ -84,17 +90,18 @@ def _CheckColorFormat(input_api, output_api):
def _CheckColorReferences(input_api, output_api):
"""Checks no (A)RGB values are defined outside color_palette.xml."""
"""Checks no (A)RGB values are defined outside colors.xml."""
errors = []
warnings = []
for f in IncludedFiles(input_api):
if f.LocalPath() == helpers.COLOR_PALETTE_RELATIVE_PATH:
if (f.LocalPath().endswith('/colors.xml') or
f.LocalPath().endswith('/color_palette.xml')):
continue
# Ignore new references in vector/shape drawable xmls
contents = input_api.ReadFile(f)
is_vector_drawable = '<vector' in contents or '<shape' in contents
for line_number, line in f.ChangedContents():
if helpers.COLOR_PATTERN.search(line):
if COLOR_PATTERN.search(line):
issue = ' %s:%d\n \t%s' % (f.LocalPath(), line_number, line.strip())
if is_vector_drawable:
warnings.append(issue)
......@@ -106,7 +113,7 @@ def _CheckColorReferences(input_api, output_api):
'''
Android Color Reference Check failed:
Your new code added new color references that are not color resources from
ui/android/java/res/values/color_palette.xml, listed below.
chrome/android/java/res/values/colors.xml, listed below.
This is banned, please use the existing color resources or create a new
color resource in colors.xml, and reference the color by @color/....
......@@ -119,7 +126,7 @@ def _CheckColorReferences(input_api, output_api):
'''
Android Color Reference Check warning:
Your new code added new color references that are not color resources from
ui/android/java/res/values/color_palette.xml, listed below.
chrome/android/java/res/values/colors.xml, listed below.
This is typically not needed even in vector/shape drawables. Please consider
using an existing color resources if possible.
......@@ -135,22 +142,23 @@ def _CheckColorReferences(input_api, output_api):
def _CheckDuplicateColors(input_api, output_api):
"""Checks colors defined by (A)RGB values in color_palette.xml are unique."""
"""Checks colors defined by (A)RGB values in colors.xml are unique."""
errors = []
for f in IncludedFiles(input_api):
if f.LocalPath() != helpers.COLOR_PALETTE_RELATIVE_PATH:
if not (f.LocalPath().endswith('/colors.xml')
or f.LocalPath().endswith('/color_palette.xml')):
continue
colors = defaultdict(int)
contents = input_api.ReadFile(f)
# Get count for each color defined.
for line in contents.splitlines(False):
color = helpers.COLOR_PATTERN.search(line)
color = COLOR_PATTERN.search(line)
if color:
colors[color.group(2)] += 1
# Check duplicates in changed contents.
for line_number, line in f.ChangedContents():
color = helpers.COLOR_PATTERN.search(line)
color = COLOR_PATTERN.search(line)
if color and colors[color.group(2)] > 1:
errors.append(
' %s:%d\n \t%s' % (f.LocalPath(), line_number, line.strip()))
......@@ -159,7 +167,7 @@ def _CheckDuplicateColors(input_api, output_api):
'''
Android Duplicate Color Declaration Check failed:
Your new code added new colors by (A)RGB values that are already defined in
ui/android/java/res/values/color_palette.xml, listed below.
chrome/android/java/res/values/colors.xml, listed below.
This is banned, please reference the existing color resource from colors.xml
using @color/... and if needed, give the existing color resource a more
......@@ -171,50 +179,12 @@ def _CheckDuplicateColors(input_api, output_api):
return []
def _CheckSemanticColorsReferences(input_api, output_api):
"""
Checks colors defined in semantic_colors.xml only referencing
resources in color_palette.xml
"""
color_palette = _colorXml2Dict(
input_api.ReadFile(helpers.COLOR_PALETTE_PATH))
errors = []
for f in IncludedFiles(input_api):
if not f.LocalPath().endswith('/semantic_colors.xml'):
continue
for line_number, line in f.ChangedContents():
r = helpers.COLOR_REFERENCE_PATTERN.search(line)
if not r:
continue
color = r.group()
if _removePrefix(color) not in color_palette:
errors.append(
' %s:%d\n \t%s' % (f.LocalPath(), line_number, line.strip()))
if errors:
return [output_api.PresubmitError(
'''
Android Semantic Color Reference Check failed:
Your new color values added in semantic_colors are not defined in
ui/android/java/res/values/color_palette.xml, listed below.
This is banned. Colors in semantic colors can only reference
the existing color resource from color_palette.xml.
See https://crbug.com/775198 for more information.
''',
errors)]
return []
def _CheckXmlNamespacePrefixes(input_api, output_api):
"""Checks consistency of prefixes used for XML namespace names."""
errors = []
for f in IncludedFiles(input_api):
for line_number, line in f.ChangedContents():
xml_app_namespace = helpers.XML_APP_NAMESPACE_PATTERN.search(line)
xml_app_namespace = XML_APP_NAMESPACE_PATTERN.search(line)
if xml_app_namespace and not xml_app_namespace.group(1) == 'app':
errors.append(
' %s:%d\n \t%s' % (f.LocalPath(), line_number, line.strip()))
......@@ -234,7 +204,6 @@ def _CheckXmlNamespacePrefixes(input_api, output_api):
return []
### text appearance below ###
def _CheckTextAppearance(input_api, output_api):
"""Checks text attributes are only used for text appearance styles in XMLs."""
text_attributes = [
......@@ -250,7 +219,7 @@ def _CheckTextAppearance(input_api, output_api):
invalid_styles = []
for style in root.findall('style') + root.findall('.//style'):
name = style.get('name')
is_text_appearance = helpers.TEXT_APPEARANCE_STYLE_PATTERN.search(name)
is_text_appearance = TEXT_APPEARANCE_STYLE_PATTERN.search(name)
item = style.find(".//item[@name='"+attribute+"']")
if is_text_appearance is None and item is not None:
invalid_styles.append(name)
......@@ -332,17 +301,3 @@ def _CheckNewTextAppearance(input_api, output_api):
''',
errors)]
return []
### helpers ###
def _colorXml2Dict(content):
dct = dict()
tree = ET.fromstring(content)
for child in tree:
dct[child.attrib['name']] = child.text
return dct
def _removePrefix(color, prefix = '@color/'):
if color.startswith(prefix):
return color[len(prefix):]
return color
\ No newline at end of file
tools/android/checkxmlstyle/checkxmlstyle_test.py
View file @ 02aa4dda
......@@ -8,7 +8,6 @@ import sys
import unittest
import checkxmlstyle
import helpers
sys.path.append(os.path.dirname(os.path.dirname(os.path.dirname(
os.path.dirname(os.path.abspath(__file__))))))
......@@ -128,25 +127,11 @@ class ColorReferencesTest(unittest.TestCase):
def testValidReferenceInColorResources(self):
lines = ['<color name="color1">#61000000</color>']
mock_input_api = MockInputApi()
mock_input_api.files = [
MockFile(helpers.COLOR_PALETTE_RELATIVE_PATH, lines)]
mock_input_api.files = [MockFile('chrome/java/res_test/colors.xml', lines)]
errors = checkxmlstyle._CheckColorReferences(
mock_input_api, MockOutputApi())
self.assertEqual(0, len(errors))
def testReferenceInSemanticColors(self):
mock_input_api = MockInputApi()
mock_input_api.files = [
MockFile(helpers.COLOR_PALETTE_PATH,
['<resources><color name="a">#f0f0f0</color></resources>']),
MockFile('ui/android/java/res/values/semantic_colors.xml',
['<color name="b">@color/hello<color>',
'<color name="c">@color/a<color>'])
]
errors = checkxmlstyle._CheckSemanticColorsReferences(
mock_input_api, MockOutputApi())
self.assertEqual(1, len(errors))
class DuplicateColorsTest(unittest.TestCase):
......@@ -154,15 +139,14 @@ class DuplicateColorsTest(unittest.TestCase):
lines = ['<color name="color1">#61000000</color>',
'<color name="color2">#61000000</color>']
mock_input_api = MockInputApi()
mock_input_api.files = [
MockFile(helpers.COLOR_PALETTE_RELATIVE_PATH, lines)]
mock_input_api.files = [MockFile('chrome/java/res_test/colors.xml', lines)]
errors = checkxmlstyle._CheckDuplicateColors(
mock_input_api, MockOutputApi())
self.assertEqual(1, len(errors))
self.assertEqual(2, len(errors[0].items))
self.assertEqual(' %s:1' % helpers.COLOR_PALETTE_RELATIVE_PATH,
self.assertEqual(' chrome/java/res_test/colors.xml:1',
errors[0].items[0].splitlines()[0])
self.assertEqual(' %s:2' % helpers.COLOR_PALETTE_RELATIVE_PATH,
self.assertEqual(' chrome/java/res_test/colors.xml:2',
errors[0].items[1].splitlines()[0])
def testSucess(self):
......
tools/android/checkxmlstyle/helpers.py deleted 100644 → 0
View file @ 06420557
# Copyright (c) 2019 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
import os
import re
_SRC_ROOT = os.path.abspath(
os.path.join(os.path.dirname(__file__), '..', '..', '..'))
COLOR_PALETTE_RELATIVE_PATH = 'ui/android/java/res/values/color_palette.xml'
COLOR_PALETTE_PATH = os.path.join(_SRC_ROOT, COLOR_PALETTE_RELATIVE_PATH)
SEMANTIC_COLORS_PATH = os.path.join(_SRC_ROOT,
'ui/android/java/res/values/semantic_colors.xml')
COLOR_PATTERN = re.compile(r'(>|")(#[0-9A-Fa-f]+)(<|")')
VALID_COLOR_PATTERN = re.compile(
r'^#([0-9A-F][0-9A-E]|[0-9A-E][0-9A-F])?[0-9A-F]{6}$')
XML_APP_NAMESPACE_PATTERN = re.compile(
r'xmlns:(\w+)="http://schemas.android.com/apk/res-auto"')
TEXT_APPEARANCE_STYLE_PATTERN = re.compile(r'^TextAppearance\.')
INCLUDED_PATHS = [
r'^(chrome|ui|components|content)[\\/](.*[\\/])?java[\\/]res.+\.xml$'
]
# TODO(lazzzis): check color references in java source files
COLOR_REFERENCE_PATTERN = re.compile('''
@color/ # starts with '@color'
[\w|_]+ # color name is only composed of numbers, letters and underscore
''', re.VERBOSE)
ui/android/java/res/values/color_palette.xml
View file @ 02aa4dda
<?xml version="1.0" encoding="utf-8"?>
<!-- Copyright 2019 The Chromium Authors. All rights reserved.
Use of this source code is governed by a BSD-style license that can be
found in the LICENSE file.
-->
<resources xmlns:tools="http://schemas.android.com/tools">
<!-- Modern color palette -->
<color name="modern_white">@android:color/white</color>
<color name="modern_blue_300">#8AB4F8</color>
<color name="modern_blue_300_alpha_55">#5A7198</color>
<color name="modern_blue_600">#1A73E8</color>
<color name="modern_blue_600_alpha_65">#6AA4F0</color>
<color name="modern_blue_700">#3367D6</color>
<color name="modern_blue_800" tools:ignore="UnusedResources">#185ABC</color>
<color name="modern_grey_50" tools:ignore="UnusedResources">#F8F9FA</color>
......@@ -22,7 +14,7 @@
<color name="modern_grey_300_alpha_38">#61DADCE0</color>
<color name="modern_grey_400" tools:ignore="UnusedResources">#BDC1C6</color>
<color name="modern_grey_500" tools:ignore="UnusedResources">#9AA0A6</color>
<color name="modern_grey_600">#80868B</color>
<color name="modern_grey_600" tools:ignore="UnusedResources">#80868B</color>
<color name="modern_grey_700">#5F6368</color>
<color name="modern_grey_800" tools:ignore="UnusedResources">#3C4043</color>
<color name="modern_grey_800_alpha_38" tools:ignore="UnusedResources">#613C4043</color>
......@@ -40,12 +32,63 @@
<color name="white_alpha_12" tools:ignore="UnusedResources">#1FFFFFFF</color>
<color name="white_alpha_20" tools:ignore="UnusedResources">#33FFFFFF</color>
<color name="white_alpha_38" tools:ignore="UnusedResources">#61FFFFFF</color>
<color name="white_alpha_50">#80FFFFFF</color>
<color name="white_alpha_50" tools:ignore="UnusedResources">#80FFFFFF</color>
<color name="white_alpha_70">#B3FFFFFF</color>
<color name="google_red_300">#F28B82</color>
<color name="google_red_600">#D93025</color>
<color name="google_red_300" tools:ignore="UnusedResources">#F28B82</color>
<color name="google_red_600" tools:ignore="UnusedResources">#D93025</color>
<color name="google_green_300" tools:ignore="UnusedResources">#81C995</color>
<color name="google_green_600" tools:ignore="UnusedResources">#1E8E3E</color>
<!-- Semantic color references that don't change on night mode. -->
<!-- Common icon colors for drawables. -->
<color name="default_icon_color_dark">@color/modern_grey_800</color>
<color name="default_icon_color_dark_disabled">@color/modern_grey_800_alpha_38</color>
<color name="default_icon_color_white" tools:ignore="UnusedResources">
@android:color/white
</color>
<color name="default_icon_color_white_disabled" tools:ignore="UnusedResources">
@color/white_alpha_50
</color>
<color name="default_icon_color_white_pressed" tools:ignore="UnusedResources">
@color/white_alpha_50
</color>
<color name="default_icon_color_secondary_dark">@color/modern_grey_600</color>
<color name="default_icon_color_secondary_light">@color/white_alpha_70</color>
<!-- Common text colors -->
<color name="default_text_color_dark">@color/modern_grey_900</color>
<color name="default_text_color_light">@android:color/white</color>
<color name="default_text_color_dark_secondary" tools:ignore="UnusedResources">@color/modern_grey_700</color>
<color name="highlight_color_on_light_text" tools:ignore="UnusedResources">#5A7198</color>
<color name="highlight_color_on_dark_text" tools:ignore="UnusedResources">#6AA4F0</color>
<color name="default_red_light" tools:ignore="UnusedResources">@color/google_red_300</color>
<color name="default_red_dark" tools:ignore="UnusedResources">@color/google_red_600</color>
<color name="default_green_light" tools:ignore="UnusedResources">@color/google_green_300</color>
<color name="default_green_dark" tools:ignore="UnusedResources">@color/google_green_600</color>
<color name="google_green_300">#81C995</color>
<color name="google_green_600">#1E8E3E</color>
<!-- Common background colors. -->
<color name="default_bg_color_light">@android:color/white</color>
<color name="default_bg_color_dark" tools:ignore="UnusedResources">
@color/modern_grey_900
</color>
<color name="default_bg_color_dark_elev_1" tools:ignore="UnusedResources">
@color/modern_grey_900_with_grey_200_alpha_5
</color>
<color name="default_bg_color_dark_elev_2" tools:ignore="UnusedResources">
@color/modern_grey_900_with_grey_200_alpha_8
</color>
<color name="default_bg_color_dark_elev_3" tools:ignore="UnusedResources">
@color/modern_grey_900_with_grey_200_alpha_11
</color>
<color name="default_bg_color_dark_elev_4" tools:ignore="UnusedResources">
@color/modern_grey_900_with_grey_200_alpha_12
</color>
<color name="divider_bg_color_dark">@color/modern_grey_300</color>
<color name="divider_bg_color_light">@color/white_alpha_12</color>
</resources>
ui/android/java/res/values/semantic_colors.xml deleted 100644 → 0
View file @ 06420557
<?xml version="1.0" encoding="utf-8"?>
<!-- Copyright 2019 The Chromium Authors. All rights reserved.
Use of this source code is governed by a BSD-style license that can be
found in the LICENSE file.
-->
<resources xmlns:tools="http://schemas.android.com/tools">
<!-- Semantic color references that don't change on night mode. -->
<!-- Common icon colors for drawables. -->
<color name="default_icon_color_dark">@color/modern_grey_800</color>
<color name="default_icon_color_dark_disabled">@color/modern_grey_800_alpha_38</color>
<color name="default_icon_color_white" tools:ignore="UnusedResources">
@color/modern_white
</color>
<color name="default_icon_color_white_disabled" tools:ignore="UnusedResources">
@color/white_alpha_50
</color>
<color name="default_icon_color_white_pressed" tools:ignore="UnusedResources">
@color/white_alpha_50
</color>
<color name="default_icon_color_secondary_dark">@color/modern_grey_600</color>
<color name="default_icon_color_secondary_light">@color/white_alpha_70</color>
<!-- Common text colors -->
<color name="default_text_color_dark">@color/modern_grey_900</color>
<color name="default_text_color_light">@color/modern_white</color>
<color name="default_text_color_dark_secondary" tools:ignore="UnusedResources">@color/modern_grey_700</color>
<color name="highlight_color_on_light_text" tools:ignore="UnusedResources">@color/modern_blue_300_alpha_55</color>
<color name="highlight_color_on_dark_text" tools:ignore="UnusedResources">@color/modern_blue_600_alpha_65</color>
<color name="default_red_light" tools:ignore="UnusedResources">@color/google_red_300</color>
<color name="default_red_dark" tools:ignore="UnusedResources">@color/google_red_600</color>
<color name="default_green_light" tools:ignore="UnusedResources">@color/google_green_300</color>
<color name="default_green_dark" tools:ignore="UnusedResources">@color/google_green_600</color>
<!-- Common background colors. -->
<color name="default_bg_color_light">@color/modern_white</color>
<color name="default_bg_color_dark">@color/modern_grey_900</color>
<color name="default_bg_color_dark_elev_1" tools:ignore="UnusedResources">
@color/modern_grey_900_with_grey_200_alpha_5
</color>
<color name="default_bg_color_dark_elev_2" tools:ignore="UnusedResources">
@color/modern_grey_900_with_grey_200_alpha_8
</color>
<color name="default_bg_color_dark_elev_3" tools:ignore="UnusedResources">
@color/modern_grey_900_with_grey_200_alpha_11
</color>
<color name="default_bg_color_dark_elev_4" tools:ignore="UnusedResources">
@color/modern_grey_900_with_grey_200_alpha_12
</color>
<color name="divider_bg_color_dark">@color/modern_grey_300</color>
<color name="divider_bg_color_light">@color/white_alpha_12</color>
</resources>
\ No newline at end of file
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