Issue 3199 - "Firefox" string is displayed on several error messages

mozharness/abb/transform_locales.py

 # This file is part of Adblock Plus
 # Copyright (C) 2006-2015 Eyeo GmbH
 #
 # Adblock Plus is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as
 # published by the Free Software Foundation.
 #
 # Adblock Plus is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # 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,3}(?:-[A-Z]{2})?)$")
 _VALUES_LOCALE_RE = re.compile("^values-([a-z]{2,3}(?:-r[A-Z]{2})?)$")
 
-_SHORTNAME_RE = re.compile("^<ShortName>(.*)</ShortName>$")

[René Jeschke, 2016/07/26 12:49:31]

Moving this line around is a unrelated change.

 _SEARCH_PROPS_RE = re.compile("^browser\.search\."
                               "(defaultenginename|order\.).*$")
-
-_PROPERTY_RE = re.compile("^(([^=]*)=)(.*)$")
-_ENTITY_RE = re.compile("^(\s*<!ENTITY\s*([^\"\s]*)\s*\")(.*)(\">)$")
-_STRING_RE = re.compile("^(\s*<string name=\"([^\"]*)\">)(.*)(</string>)$")
+_SHORTNAME_RE = re.compile("^<ShortName>(.*)</ShortName>$")
+
+_PROPERTY_FORMAT_RE = re.compile("^(([^=]*)=)(.*)$")
+_ENTITY_FORMAT_RE = re.compile("^(\s*<!ENTITY\s*([^\"\s]*)\s*\")(.*)(\">)$")
+_STRING_FORMAT_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"
@@ skipping 12 matching lines @@
               "yahoo",
               "google",
               "wikipedia",
               "amazon"
               ]
 }
 
 _FIREFOX_REPLACE_STR = "Firefox"
 _ABB_REPLACEMENT_STR = "Adblock Browser"
 
+# Some string values that contain Firefox such as 'Firefox Sync' shouldn't be
+# replaced, so we keep a list of ids that are exceptions
 _ENTITY_EXCEPTIONS = [
     "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

[René Jeschke, 2016/07/26 12:49:31]

Why do you change a docstring into comment here? Same below.

+    """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=[]):
-    match = replacement_re.match(str)
+def _replace_in_value(format_re, str, old, new, exceptions=[]):
+    match = format_re.match(str)
     if match and match.lastindex > 2:
-        if match.group(2) not in exceptions and old in match.group(3):
-            new_str = match.group(1) + match.group(3).replace(old, new)
+        # The prefix contains all characters that precedes the value, including
+        # the id/key
+        str_value_prefix = match.group(1)
+        str_id = match.group(2)
+        str_value = match.group(3)
+        if str_id not in exceptions and old in str_value:
+            new_str = str_value_prefix + str_value.replace(old, new)
             if match.lastindex == 4:
-                new_str = new_str + match.group(4)
+                # The suffix contains all characters that succeeds the value
+                str_value_suffix = match.group(4)
+                new_str = new_str + str_value_suffix
             return new_str
     return None
 
 
 def _write_lines(filename, lines):
-    # Writes lines into file appending \n
+    """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)
@@ skipping 71 matching lines @@
     # Append default search engine name
     props.append("browser.search.defaultenginename=%s" % engine_order[0])
 
     # 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'
-        in appstrings.properties"""
+    # Replaces ocurrences of 'Firefox' by 'Adblock Browser' 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)
+            replacement = _replace_in_value(_PROPERTY_FORMAT_RE, line,
+                                            _FIREFOX_REPLACE_STR,
+                                            _ABB_REPLACEMENT_STR)
             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
+    # 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)
+            replacement = _replace_in_value(_ENTITY_FORMAT_RE, line,
+                                            _FIREFOX_REPLACE_STR,
+                                            _ABB_REPLACEMENT_STR,
+                                            _ENTITY_EXCEPTIONS)
             if replacement:
                 line = replacement
                 replacement_count += 1
             else:
-                replacement = _replace_str_re(line, _FIREFOX_REPLACE_STR,
-                                              _ABB_REPLACEMENT_STR, _STRING_RE)
+                replacement = _replace_in_value(_STRING_FORMAT_RE, line,
+                                                _FIREFOX_REPLACE_STR,
+                                                _ABB_REPLACEMENT_STR)
                 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))
 
@@ skipping 28 matching lines @@
         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 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)