Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: mozharness/abb/transform_locales.py

Issue 29348650: Issue 3199 - "Firefox" string is displayed on several error messages (Closed)
Patch Set: Removed unrelated changes and added 2 more entity exceptions Created July 28, 2016, 11:08 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: mozharness/abb/transform_locales.py
===================================================================
--- a/mozharness/abb/transform_locales.py
+++ b/mozharness/abb/transform_locales.py
@@ -11,25 +11,37 @@
# 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/>.
import os
import re
-_LOCALE_RE = re.compile("^[a-z]{2}(-[A-Z]{2})?$")
+_LOCALE_RE = re.compile("^([a-z]{2,3}(?:-[A-Z]{2})?)$")
+_VALUES_LOCALE_RE = re.compile("^values-([a-z]{2,3}(?:-r[A-Z]{2})?)$")
Felix Dahlke 2016/12/13 13:06:05 What's the "r" for here?
Felix Dahlke 2016/12/13 13:11:15 Whoops, I was planning to remove this comment once
+
_SEARCH_PROPS_RE = re.compile("^browser\.search\."
"(defaultenginename|order\.).*$")
_SHORTNAME_RE = re.compile("^<ShortName>(.*)</ShortName>$")
+_PROPERTY_RE = re.compile("^(([^=]*)=)(.*)$")
+_ENTITY_RE = re.compile("^(\s*<!ENTITY\s*([^\"\s]*)\s*\")(.*)(\">)$")
+_STRING_RE = re.compile("^(\s*<string name=\"([^\"]*)\">)(.*)(</string>)$")
+
+_CHROME_PATH = os.path.join("dist", "bin", "chrome")
+_RES_PATH = os.path.join("mobile", "android", "base", "res")
+
_SEARCHPLUGINS_PATH = os.path.join("browser", "searchplugins")
_LIST_TXT_PATH = os.path.join(_SEARCHPLUGINS_PATH, "list.txt")
_REGION_PROPS_PATH = os.path.join("browser", "region.properties")
+_APPSTRINGS_PROPS_PATH = os.path.join("browser", "appstrings.properties")
+_STRINGS_XML_PATH = "strings.xml"
+
_DEFAULT_LOCALE = "en-US"
_SEARCH_ENGINE_ORDER = {
"en-US": ["duckduckgo",
"yahoo",
"google",
"wikipedia",
"amazon"
@@ -38,96 +50,146 @@ import re
"duckduckgo",
"yahoo",
"google",
"wikipedia",
"amazon"
]
}
+_FIREFOX_REPLACE_STR = "Firefox"
+_ABB_REPLACEMENT_STR = "Adblock Browser"
+
+_ENTITY_EXCEPTIONS = [
Felix Dahlke 2016/12/13 13:06:05 Why do we exclude these? Would be helpful to have
diegocarloslima 2016/12/21 13:21:48 These are excluded because they relate to some fea
+ "overlay_no_synced_devices",
+ "home_remote_tabs_need_to_sign_in",
+ "home_remote_tabs_need_to_finish_migrating",
+ "home_remote_tabs_need_to_verify",
+ "syncBrand.fullName.label",
+ "sync.subtitle.connectlocation2.label",
+ "sync.subtitle.failmultiple.label",
+ "fxaccount_full_label",
+ "fxaccount_create_account_header2",
+ "fxaccount_create_account_policy_text2",
+ "fxaccount_status_header2",
+ "fxaccount_status_needs_finish_migrating",
+ "fxaccount_remove_account_dialog_title",
+ "fxaccount_remove_account_toast",
+ "fxaccount_account_type_label",
+]
+
+
+def _check_path_exists(path, logger):
+ if not os.path.exists(path):
+ logger.fatal("'%s' does not exist" % path)
+
+
+def _get_locales_from_path(path, locale_re):
+ locales = []
+ for dir_name in next(os.walk(path))[1]:
+ match = locale_re.match(dir_name)
+ if match:
+ locales.append(match.group(1))
+ locales.sort
+ return locales
+
def _get_shortname_from_id(needle, engine_ids, engine_names):
"""Fuzzy finds needle in engine_ids and returns ShortName"""
for engine in engine_ids:
if engine.startswith(needle):
return engine_names[engine]
return None
+def _replace_str_re(str, old, new, replacement_re, exceptions=[]):
Felix Dahlke 2016/12/13 13:06:06 Took me a little while to wrap my head around what
diegocarloslima 2016/12/21 13:21:48 Acknowledged.
+ match = replacement_re.match(str)
+ if match and match.lastindex > 2:
+ if match.group(2) not in exceptions and old in match.group(3):
Felix Dahlke 2016/12/13 13:06:05 Shouldn't `old` be `in match.group(2)` rather than
diegocarloslima 2016/12/21 13:21:48 Actually match.group(2) matches the id/key of the
+ new_str = match.group(1) + match.group(3).replace(old, new)
+ if match.lastindex == 4:
+ new_str = new_str + match.group(4)
+ return new_str
+ return None
+
+
def _write_lines(filename, lines):
"""Writes lines into file appending \\n"""
with open(filename, "w") as fd:
for l in lines:
fd.write("%s\n" % l)
def _transform_locale(locale, path, logger):
logger.info("Processing locale '%s'..." % locale)
# Check for list.txt existence
list_file_path = os.path.join(path, _LIST_TXT_PATH)
- if not os.path.exists(list_file_path):
- logger.fatal("Missing 'list.txt' for locale '%s'" % locale)
+ _check_path_exists(list_file_path, logger)
# Check for region.properties existence
region_file_path = os.path.join(path, _REGION_PROPS_PATH)
- if not os.path.exists(region_file_path):
- logger.fatal("Missing 'region.properties' for locale '%s'" % locale)
+ _check_path_exists(region_file_path, logger)
+
+ # Check for appstrings.properties existence
+ appstrings_file_path = os.path.join(path, _APPSTRINGS_PROPS_PATH)
+ _check_path_exists(appstrings_file_path, logger)
# Get whitelist and build regex
whitelist = _SEARCH_ENGINE_ORDER.get(locale,
_SEARCH_ENGINE_ORDER[_DEFAULT_LOCALE])
white_re = re.compile("^(%s).*$" % "|".join(whitelist))
# Read engine IDs from list.txt, discard engines not on whitelist
engine_ids = []
+ removed_engine_ids = []
with open(list_file_path, "r") as fd:
for line in fd:
line = line.strip()
if len(line) > 0:
if white_re.match(line):
engine_ids.append(line)
else:
- logger.info("Removing '%s'" % line)
+ removed_engine_ids.append(line)
# Make sure we still have search engines left
if len(engine_ids) == 0:
logger.fatal("No search engines left over for '%s'" % locale)
# 'Parse' XML to get matching 'ShortName' for all engine IDs
engine_names = {}
for eid in engine_ids:
xml_file_path = os.path.join(path, _SEARCHPLUGINS_PATH, "%s.xml" % eid)
- if os.path.exists(xml_file_path):
- short_name = None
- with open(xml_file_path, "r") as fd:
- for line in fd:
- line = line.strip()
- match = _SHORTNAME_RE.match(line)
- if match:
- short_name = match.group(1).strip()
+ _check_path_exists(xml_file_path, logger)
+ short_name = None
+ with open(xml_file_path, "r") as fd:
+ for line in fd:
+ line = line.strip()
+ match = _SHORTNAME_RE.match(line)
+ if match:
+ short_name = match.group(1).strip()
- if not short_name:
- logger.fatal("No ShortName defined for '%s' in '%s" %
- (eid, locale))
- engine_names[eid] = short_name
- else:
- logger.fatal("XML definiton for '%s' in '%s' missing" %
+ if not short_name:
+ logger.fatal("No ShortName defined for '%s' in '%s" %
(eid, locale))
+ engine_names[eid] = short_name
- logger.info("Remaining engine IDs: %s" % ", ".join(engine_ids))
+ logger.info("Removed search engine IDs: %s" %
+ ", ".join(removed_engine_ids))
+ logger.info("Remaining search engine IDs: %s" % ", ".join(engine_ids))
# Create search engine order with real engine names
engine_order = []
for eid in whitelist:
sn = _get_shortname_from_id(eid, engine_ids, engine_names)
if sn:
engine_order.append(sn)
- logger.info("Resulting ordered list: %s" % (", ".join(engine_order)))
+ logger.info("Resulting search engine ordered list: %s" %
+ (", ".join(engine_order)))
# Read region.properties and remove browser.search.* lines
props = []
with open(region_file_path, "r") as fd:
for line in fd:
line = line.rstrip("\r\n")
if not _SEARCH_PROPS_RE.match(line.strip()):
props.append(line)
@@ -137,41 +199,100 @@ def _transform_locale(locale, path, logg
# Append search engine order
for i in range(0, min(5, len(engine_order))):
props.append("browser.search.order.%d=%s" % (i + 1, engine_order[i]))
# Write back region.properties
_write_lines(region_file_path, props)
+ """Replaces ocurrences of 'Firefox' by 'Adblock Browser'
Felix Dahlke 2016/12/13 13:06:06 Shouldn't this be a normal comment rather than a d
diegocarloslima 2016/12/21 13:21:48 Actually, I had to make it a multiline comment to
+ in 'appstrings.properties'"""
+ lines = []
+ replacement_count = 0
+
+ with open(appstrings_file_path, "r") as fd:
+ for line in fd:
+ line = line.rstrip("\r\n")
+ replacement = _replace_str_re(line, _FIREFOX_REPLACE_STR,
+ _ABB_REPLACEMENT_STR, _PROPERTY_RE)
+ if replacement:
+ line = replacement
+ replacement_count += 1
+ lines.append(line)
+
+ # Apply changes to appstrings.properties
+ _write_lines(appstrings_file_path, lines)
+ logger.info("Replaced %d ocurrences of %s in 'appstrings.properties'" %
+ (replacement_count, _FIREFOX_REPLACE_STR))
+
+
+def _transform_values_locale(locale, path, logger):
+ logger.info("Processing values-%s..." % locale)
+
+ # Check for strings.xml existence
+ strings_file_path = os.path.join(path, _STRINGS_XML_PATH)
+ _check_path_exists(strings_file_path, logger)
+
+ # Replaces ocurrences of 'Firefox' by 'Adblock Browser' in 'strings.xml'
+ lines = []
+ replacement_count = 0
+
+ with open(strings_file_path, "r") as fd:
+ for line in fd:
+ line = line.rstrip("\r\n")
+ replacement = _replace_str_re(line, _FIREFOX_REPLACE_STR,
+ _ABB_REPLACEMENT_STR, _ENTITY_RE,
+ _ENTITY_EXCEPTIONS)
+ if replacement:
+ line = replacement
+ replacement_count += 1
+ else:
+ replacement = _replace_str_re(line, _FIREFOX_REPLACE_STR,
+ _ABB_REPLACEMENT_STR, _STRING_RE)
+ if replacement:
+ line = replacement
+ replacement_count += 1
+ lines.append(line)
+
+ # Apply changes to strings.xml
+ _write_lines(strings_file_path, lines)
+ logger.info("Replaced %d ocurrences of %s in 'strings.xml'" %
+ (replacement_count, _FIREFOX_REPLACE_STR))
+
class MinimalLogger:
def info(self, s):
print "INFO: %s" % s
def error(self, s):
print "ERROR: %s" % s
def fatal(self, s):
print "FATAL: %s" % s
exit(1)
def transform_locales(obj_dir, logger=MinimalLogger()):
- chrome_path = os.path.join(obj_dir, "dist", "bin", "chrome")
- if not os.path.exists(chrome_path):
- logger.fatal("'%s' does not exist" % obj_dir)
+ chrome_path = os.path.join(obj_dir, _CHROME_PATH)
+ _check_path_exists(chrome_path, logger)
- locales = []
- for p in next(os.walk(chrome_path))[1]:
- if _LOCALE_RE.match(p):
- locales.append(p)
- locales.sort()
+ res_path = os.path.join(obj_dir, _RES_PATH)
+ _check_path_exists(res_path, logger)
- logger.info("Found %d locales" % len(locales))
+ locales = _get_locales_from_path(chrome_path, _LOCALE_RE)
+ values_locales = _get_locales_from_path(res_path, _VALUES_LOCALE_RE)
+
+ locales_found_msg = "Found %d locales in %s"
+ logger.info(locales_found_msg % (len(locales), chrome_path))
+ logger.info(locales_found_msg % (len(values_locales), res_path))
for locale in locales:
locale_path = os.path.join(chrome_path, locale, "locale", locale)
if os.path.exists(locale_path):
_transform_locale(locale, locale_path, logger)
else:
- logger.error("Missing 'locale' folder for '%s'" % locale)
+ logger.error("Missing folder for locale '%s' in path: %s" %
+ (locale, locale_path))
+ for locale in values_locales:
+ locale_path = os.path.join(res_path, "values-" + locale)
+ _transform_values_locale(locale, locale_path, logger)
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld