[Android] Warning if Left/Right attributes are used in layout and style xmls.

build/android/gyp/generate_v14_resources.py

Index: build/android/gyp/generate_v14_resources.py
diff --git a/build/android/gyp/generate_v14_resources.py b/build/android/gyp/generate_v14_resources.py
index a07a42a88d10a0f6c491d2e11bcc09173b852ead..7d0180d1651e7b8605d28b5228935a2e8df5985c 100755
--- a/build/android/gyp/generate_v14_resources.py
+++ b/build/android/gyp/generate_v14_resources.py
@@ -27,8 +27,10 @@ import xml.dom.minidom as minidom
 
 from util import build_utils
 
+# Note that we are assumping 'android:' is an alias of

[newt (away), 2013/05/10 01:15:49]

"assuming"

[Kibeom Kim (inactive), 2013/05/10 02:14:27]

Done.

+# the namespace 'http://schemas.android.com/apk/res/android'.
 
-ATTRIBUTE_NAMESPACE = 'http://schemas.android.com/apk/res/android'
+GRAVITY_ATTRIBUTES = ('android:gravity', 'android:layout_gravity')
 
 # Almost all the attributes that has "Start" or "End" in
 # its name should be mapped.
@@ -45,12 +47,11 @@ ATTRIBUTES_TO_MAP = {'paddingStart' : 'paddingLeft',
                      'layout_alignParentEnd' : 'layout_alignParentRight',
                      'layout_toEndOf' : 'layout_toRightOf'}
 
-ATTRIBUTES_TO_MAP_NS = {}
+ATTRIBUTES_TO_MAP = dict(['android:' + k, 'android:' + v] for k, v
+                         in ATTRIBUTES_TO_MAP.iteritems())
 
-for k, v in ATTRIBUTES_TO_MAP.items():
-  ATTRIBUTES_TO_MAP_NS[(ATTRIBUTE_NAMESPACE, k)] = (ATTRIBUTE_NAMESPACE, v)
-
-ATTRIBUTES_TO_MAP_NS_VALUES = set(ATTRIBUTES_TO_MAP_NS.values())
+ATTRIBUTES_TO_MAP_REVERSED = dict([v,k] for k, v
+                                  in ATTRIBUTES_TO_MAP.iteritems())
 
 
 def IterateXmlElements(node):
@@ -63,22 +64,33 @@ def IterateXmlElements(node):
       yield child_node_element
 
 
-def GenerateV14StyleResource(dom, output_file):
+def WarnDeprecatedAttribute(name, value, file):

[newt (away), 2013/05/10 01:16:43]

also: it's better to use filename instead of file, which has a built-in meaning
in python

[Kibeom Kim (inactive), 2013/05/10 02:14:27]

Done.

+  if name in ATTRIBUTES_TO_MAP_REVERSED:
+    print >> sys.stderr, ('warning: ' +file + ' should use ' +

[newt (away), 2013/05/10 01:15:49]

nit: space before "file"

[Kibeom Kim (inactive), 2013/05/10 02:14:27]

Done.

+                          ATTRIBUTES_TO_MAP_REVERSED[name] +
+                          ' instead of ' + name)
+  elif name in GRAVITY_ATTRIBUTES and ('left' in value or 'right' in value):

[newt (away), 2013/05/10 01:15:49]

elif name in GRAVITY_ATTRIBUTES and value in ('left', 'right'):

[Kibeom Kim (inactive), 2013/05/10 02:14:27]

But what if value == 'left|top' ?

[newt (away), 2013/05/10 06:13:50]

ah, good point. looks good as is.

+    print >> sys.stderr, ('warning: ' + file +
+                          ' should use start/end instead of left/right for ' +
+                          name)
+
+
+def GenerateV14StyleResource(input_file, output_file):
   """Convert style resource to API 14 compatible style resource.
 
   It's mostly a simple replacement, s/Start/Left s/End/Right,
   on the attribute names specified by <item> element.
   """
+  dom = minidom.parse(input_file)
+
   for style_element in dom.getElementsByTagName('style'):
     for item_element in style_element.getElementsByTagName('item'):
-      namespace, name = item_element.attributes['name'].value.split(':')
-      # Note: namespace == 'android' is not precise because
-      # we are looking for 'http://schemas.android.com/apk/res/android' and
-      # 'android' can be aliased to another name in layout xml files where
-      # this style is used. e.g. xmlns:android="http://crbug.com/".
-      if namespace == 'android' and name in ATTRIBUTES_TO_MAP:
-        mapped_name = ATTRIBUTES_TO_MAP[name]
-        item_element.attributes['name'] = namespace + ':' + mapped_name
+      name = item_element.attributes['name']

[newt (away), 2013/05/10 01:15:49]

add ".value" to the end of the line?

[Kibeom Kim (inactive), 2013/05/10 02:14:27]

Done. Thanks for catching this!

+      value = item_element.childNodes[0].nodeValue
+      if name in ATTRIBUTES_TO_MAP:
+        item_element.attributes['name'] = ATTRIBUTES_TO_MAP[name]
+      else:
+        WarnDeprecatedAttribute(name, value, 'xxx')

[Kibeom Kim (inactive), 2013/05/10 00:31:39]

oops, 'xxx' to file

[newt (away), 2013/05/10 01:15:49]

'xxx' -> input_file

[Kibeom Kim (inactive), 2013/05/10 02:14:27]

Done.

[Kibeom Kim (inactive), 2013/05/10 02:14:27]

Done.

 
   build_utils.MakeDirectory(os.path.dirname(output_file))
   with open(output_file, 'w') as f:
@@ -93,37 +105,19 @@ def GenerateV14LayoutResource(input_file, output_file):
   """
   dom = minidom.parse(input_file)

[newt (away), 2013/05/10 01:15:49]

you could use ElementTree and xpath here:

    tree = ElementTree.parse(input_file)
    for item in tree.findall('resources/style/items'):
        ...

[Kibeom Kim (inactive), 2013/05/10 02:14:27]

Actually, I used cElementTree before because it's the fastest built-in xml
parser http://effbot.org/zone/celementtree.htm , but then it uses
xmlns:ns0="http://schemas.android.com/apk/res/android" by default and to set
'android' instead of 'ns0' I had to call
xml.etree.ElementTree.register_namespace(...) which is a v2.7 function. So I
switched to minidom.

[newt (away), 2013/05/10 06:13:50]

ah. looks like there's a solution in python 2.6: look for
"ET._namespace_map[uri] = prefix" in
http://effbot.org/zone/element-namespaces.htm . But I think it's fine to leave
this as it is.

 
+  # Iterate all the elements' attributes to find attributes to convert.
   for element in IterateXmlElements(dom):
-    all_names = element.attributes.keysNS()
-
-    # Iterate all the attributes to find attributes to convert.
-    # Note that name variable is actually a tuple that has namespace and name.
-    # For example,
-    # name == ('http://schemas.android.com/apk/res/android', 'paddingStart')
-    for name, value in list(element.attributes.itemsNS()):
+    for name, value in list(element.attributes.items()):
+      # Convert any other API 17 Start/End attributes to Left/Right attributes.
+      # For example, from paddingStart="10dp" to paddingLeft="10dp"
       # Note: gravity attributes are not necessary to convert because
       # start/end values are backward-compatible. Explained at
       # https://plus.sandbox.google.com/+RomanNurik/posts/huuJd8iVVXY?e=Showroom
-
-      # Convert any other API 17 Start/End attributes to Left/Right attributes.
-      # For example, from paddingStart="10dp" to paddingLeft="10dp"
-      if name in ATTRIBUTES_TO_MAP_NS:
-        mapped_name = ATTRIBUTES_TO_MAP_NS[name]
-
-        # Add the new mapped attribute and remove the original attribute.
-        # For example, add paddingLeft and remove paddingStart.
-        # Note that instead of element.setAttribute(...), this is more correct.
-        # element.setAttributeNS(mapped_name[0], mapped_name[1], value)
-        # However, there is a minidom bug that doesn't print namespace set by
-        # setAttributeNS. Hence this workaround.
-        # This is a similar bug discussion about minidom namespace normalizing.
-        # http://stackoverflow.com/questions/863774/how-to-generate-xml-documents-with-namespaces-in-python
-        element.setAttribute('android:' + mapped_name[1], value)
+      if name in ATTRIBUTES_TO_MAP:
+        element.setAttribute(ATTRIBUTES_TO_MAP[name], value)
         del element.attributes[name]
-      elif name in ATTRIBUTES_TO_MAP_NS_VALUES:
-        # TODO(kkimlabs): Enable  warning once layouts have been converted
-        # print >> sys.stderror, 'Warning: layout should use xxx instead of yyy'
-        pass
+      else:
+        WarnDeprecatedAttribute(name, value, input_file)
 
   build_utils.MakeDirectory(os.path.dirname(output_file))
   with open(output_file, 'w') as f:
@@ -139,7 +133,7 @@ def GenerateV14XmlResourcesInDir(input_dir, output_dir, only_styles=False):
       dom = minidom.parse(input_file)

[newt (away), 2013/05/10 01:15:49]

instead of parsing input_file twice, I'd remove these three lines. Then, in
GenerateV14StyleResource(), check if input_file has any style elements. if it
doesn't, no need generate the output file.

[Kibeom Kim (inactive), 2013/05/10 02:14:27]

Done.

       if not dom.getElementsByTagName('style'):
         continue
-      GenerateV14StyleResource(dom, output_file)
+      GenerateV14StyleResource(input_file, output_file)
     else:
       GenerateV14LayoutResource(input_file, output_file)