Adding Julien review suggestions.

- Additionally, I have moved all remaining transform related stuff
 from transition plugin to transform class, and checked that regex
 are updated based on specification (it was a copy from original
 jQuery plugin).
- Adding some tests.

6 files changed, 149 insertions, 74 deletions

diff --git a/gwtquery-core/src/main/java/com/google/gwt/query/client/js/JsCache.java b/gwtquery-core/src/main/java/com/google/gwt/query/client/js/JsCache.java
index 2862ee0f..2f201626 100644
--- a/gwtquery-core/src/main/java/com/google/gwt/query/client/js/JsCache.java
+++ b/gwtquery-core/src/main/java/com/google/gwt/query/client/js/JsCache.java
@@ -135,7 +135,10 @@ public class JsCache extends JavaScriptObject {
   }
 
   public final native boolean isEmpty() /*-{
-    for (k in this) if (this.hasOwnProperty(k)) return false;
+    for (k in this) {
+     if (this.hasOwnProperty(k))
+       return false;
+    }
     return true;
   }-*/;
 
@@ -185,7 +188,10 @@ public class JsCache extends JavaScriptObject {
 
     var key, ret = 0;
     // Chrome in DevMode injects a property to JS objects
-    for (key in this) if (key != "__gwt_ObjectId") ret ++;
+    for (key in this) {
+      if (key != "__gwt_ObjectId")
+         ret ++;
+    }
     return ret;
   }-*/;
 
@@ -240,7 +246,10 @@ public class JsCache extends JavaScriptObject {
     var key, keys=[];
     // Chrome in DevMode sets '__gwt_ObjectId' to JS objects
     // GWT sets '$H' when calling getHashCode (see com/google/gwt/core/client/impl/Impl.java)
-    for(key in this) if (this.hasOwnProperty(key) && key != '__gwt_ObjectId' && key != '$H') keys.push(String(key));
+    for(key in this) {
+      if (this.hasOwnProperty(key) && key != '__gwt_ObjectId' && key != '$H')
+        keys.push(String(key));
+    }
     return keys;
   }-*/;
 
diff --git a/gwtquery-core/src/main/java/com/google/gwt/query/client/js/JsUtils.java b/gwtquery-core/src/main/java/com/google/gwt/query/client/js/JsUtils.java
index ecd7c6a8..14908568 100644
--- a/gwtquery-core/src/main/java/com/google/gwt/query/client/js/JsUtils.java
+++ b/gwtquery-core/src/main/java/com/google/gwt/query/client/js/JsUtils.java
@@ -306,7 +306,6 @@ public class JsUtils {
    * </pre>
    */
   public static native boolean hasProperty(JavaScriptObject o, String name)/*-{
-    return o && name in o;
     var p = name.split('.');
     for (var i in p) {
       if (!(o && p[i] in o)) return false;
diff --git a/gwtquery-core/src/main/java/com/google/gwt/query/client/plugins/effects/Transform.java b/gwtquery-core/src/main/java/com/google/gwt/query/client/plugins/effects/Transform.java
index 641a02cc..a165cf81 100644
--- a/gwtquery-core/src/main/java/com/google/gwt/query/client/plugins/effects/Transform.java
+++ b/gwtquery-core/src/main/java/com/google/gwt/query/client/plugins/effects/Transform.java
@@ -15,8 +15,13 @@
  */
 package com.google.gwt.query.client.plugins.effects;
 
+import static com.google.gwt.query.client.GQuery.browser;
+
+import com.google.gwt.dom.client.Document;
 import com.google.gwt.dom.client.Element;
+import com.google.gwt.dom.client.Style;
 import com.google.gwt.query.client.GQuery;
+import com.google.gwt.query.client.js.JsUtils;
 import com.google.gwt.regexp.shared.MatchResult;
 import com.google.gwt.regexp.shared.RegExp;
 
@@ -27,18 +32,60 @@ import java.util.Map.Entry;
 
 /**
  * A dictionary class with all the properties of an element transform
- * which is able to return the correct syntax for setting the css transform
+ * which is able to return the correct syntax for setting the CSS transform
  * property.
  */
 public class Transform  {
 
   private static final String TRANSFORM = "_t_";
 
-  protected static final RegExp transformRegex = RegExp.compile("^(scale([XYZ]|)|translate([XYZ]|3d)|rotate([XYZ]|3d)?|perspective|skew[XYZ]|x|y)$");
-  private static final RegExp transform3dRegex = RegExp.compile("^(rotate([XYZ]|3d)|perspective)$");
+  // Used to check supported properties in the browser
+  protected static final Style divStyle = Document.get().createDivElement().getStyle();
+
+  // Compute browser specific constants, public so as they are usable in plugins
+  public static final String prefix = browser.msie ? "ms" : browser.opera ? "o" : browser.mozilla ? "moz" : browser.webkit ? "webkit" : "";
+  public static final String transform = getVendorPropertyName("transform");
+  public static final String transformOrigin = getVendorPropertyName("transformOrigin");
+  // Non final for testing purposes.
+  public static boolean has3d = supportsTransform3d();
+
+  // Regular expressions based on http://www.w3schools.com/cssref/css3_pr_transform.asp
+  protected static final RegExp transformRegex = RegExp.compile("^(matrix(3d)?|(translate|scale|rotate)([XYZ]|3d)?|skew([XY])?|perspective|x|y|z)$");
+  private static final RegExp transform3dRegex = RegExp.compile("^(rotate[XY]|\\w+(Z|3d)|perspective)$");
+  private static final RegExp transformParseRegex = RegExp.compile("(\\w+)\\((.*?)\\)", "g");
 
   private HashMap<String, List<String>> map = new HashMap<String, List<String>>();
 
+  // Some browsers like HTMLUnit only support 2d transformations
+  private static boolean supportsTransform3d() {
+    if (transform == null) {
+      return false;
+    }
+    String rotate = "rotateY(1deg)";
+    divStyle.setProperty(transform, rotate);
+    rotate = divStyle.getProperty(transform);
+    return rotate != null && !rotate.isEmpty();
+  }
+
+  /**
+   * Compute the correct CSS property name for a specific browser vendor.
+   */
+  public static String getVendorPropertyName(String prop) {
+    // we prefer vendor specific names by default
+    String vendorProp =  JsUtils.camelize("-" + prefix + "-" + prop);
+    if (JsUtils.hasProperty(divStyle, vendorProp)) {
+      return vendorProp;
+    }
+    if (JsUtils.hasProperty(divStyle, prop)) {
+      return prop;
+    }
+    String camelProp = JsUtils.camelize(prop);
+    if (JsUtils.hasProperty(divStyle, camelProp)) {
+      return camelProp;
+    }
+    return null;
+  }
+
   /**
    * Return the Transform dictionary object of a element.
    */
@@ -46,6 +93,9 @@ public class Transform  {
     return getInstance(e, null);
   }
 
+  /**
+   * Return true if the propName is a valid value of the css3 transform property.
+   */
   public static boolean isTransform(String propName) {
     return transformRegex.test(propName);
   }
@@ -63,10 +113,17 @@ public class Transform  {
     return t;
   }
 
+  /**
+   * Create a new Transform dictionary setting initial values based on the
+   * string passed.
+   */
   public Transform(String s) {
     parse(s);
   }
 
+  /**
+   * Return the value of a transform property.
+   */
   public String get(String prop) {
     return listToStr(map.get(prop), ",");
   }
@@ -81,22 +138,31 @@ public class Transform  {
     return v;
   }
 
+  /**
+   * Parse a transform value as string and fills the dictionary map.
+   */
   private void parse(String s) {
     if (s != null) {
-      RegExp re = RegExp.compile("([a-zA-Z0-9]+)\\((.*?)\\)", "g");
-      for (MatchResult r = re.exec(s); r != null; r = re.exec(s)) {
+      for (MatchResult r = transformParseRegex.exec(s); r != null; r = transformParseRegex.exec(s)) {
         setFromString(r.getGroup(1), r.getGroup(2));
       }
     }
   }
 
+  /**
+   * Set a transform value or multi-value.
+   */
   public void set(String prop, String ...val) {
     setter(prop, val);
   }
 
+  /**
+   * Set a transform multi-value giving either a set of strings or
+   * just an string of values separated by comma.
+   */
   public void setFromString(String prop, String ...val) {
     if (val.length == 1) {
-      String[] vals = val[0].split("[\\s*,\\s*]");
+      String[] vals = val[0].split("[\\s,]+");
       set(prop, vals);
     } else {
       set(prop, val);
@@ -104,7 +170,7 @@ public class Transform  {
   }
 
   private void setter(String prop, String ...val) {
-    if (prop.matches("(rotate[XYZ]?|skew[XYZ])")) {
+    if (prop.matches("(rotate[XYZ]?|skew[XY])")) {
       map.put(prop, unit(val[0], "deg"));
     } else if ("scale".equals(prop)) {
       String x = val.length < 1 ? "1" : val[0];
@@ -116,42 +182,35 @@ public class Transform  {
       setter("translate", val[0], null);
     } else if ("y".equals(prop)) {
       setter("translate", null, val[0]);
+    } else if ("z".equals(prop)) {
+      setter("translate", null, null, val[0]);
     } else if (prop.matches("(translate[XYZ])")) {
       map.put(prop, unit(val[0], "px"));
     } else if ("translate".equals(prop)) {
-      if (map.get("translateX") == null) {
-        map.put("translateX", unit("0", "px"));
-      }
       if (val[0] != null) {
         map.put("translateX", unit(val[0], "px"));
       }
-      if (map.get("translateY") == null) {
-        map.put("translateY", unit("0", "px"));
-      }
       if (val.length > 1 && val[1] != null) {
         map.put("translateY", unit(val[1], "px"));
       }
-      if (map.get("translateZ") == null) {
-        map.put("translateZ", unit("0", "px"));
-      }
-      if (val.length > 2 && val[2] != null) {
-        map.put("translateZ", unit(val[0], "px"));
+      if (has3d && val.length > 2 && val[2] != null) {
+        map.put("translateZ", unit(val[2], "px"));
       }
-      map.put("translate", Arrays.asList(map.get("translateX").get(0), map.get("translateY").get(0), map.get("translateY").get(0)));
     } else {
-      map.put(prop, unit(val[0], ""));
+      map.put(prop, Arrays.asList(val));
     }
   }
 
   /**
-   * Converts the dictionary to a transition css string.
+   * Converts the dictionary to a transition css string value but
+   * excluding 3d properties if the browser only supports 2d.
    */
   public String toString() {
     // purposely using string addition, since my last tests demonstrate
     // that string addition performs better than string builders in gwt-prod.
     String ret = "";
     for (Entry<String, List<String>> e: map.entrySet()) {
-      if (Transitions.has3d || !transform3dRegex.test(e.getKey())) {
+      if (has3d || !transform3dRegex.test(e.getKey())) {
         String v = listToStr(e.getValue(), ",");
         ret += (ret.isEmpty() ? "" : " ") + e.getKey() + "(" + v + ")";
       }
diff --git a/gwtquery-core/src/main/java/com/google/gwt/query/client/plugins/effects/Transitions.java b/gwtquery-core/src/main/java/com/google/gwt/query/client/plugins/effects/Transitions.java
index b939a9b1..bff20334 100644
--- a/gwtquery-core/src/main/java/com/google/gwt/query/client/plugins/effects/Transitions.java
+++ b/gwtquery-core/src/main/java/com/google/gwt/query/client/plugins/effects/Transitions.java
@@ -28,9 +28,14 @@
  */
 package com.google.gwt.query.client.plugins.effects;
 
-import com.google.gwt.dom.client.Document;
+import static com.google.gwt.query.client.plugins.effects.Transform.getInstance;
+import static com.google.gwt.query.client.plugins.effects.Transform.getVendorPropertyName;
+import static com.google.gwt.query.client.plugins.effects.Transform.isTransform;
+import static com.google.gwt.query.client.plugins.effects.Transform.prefix;
+import static com.google.gwt.query.client.plugins.effects.Transform.transform;
+import static com.google.gwt.query.client.plugins.effects.Transform.transformOrigin;
+
 import com.google.gwt.dom.client.Element;
-import com.google.gwt.dom.client.Style;
 import com.google.gwt.query.client.Function;
 import com.google.gwt.query.client.GQuery;
 import com.google.gwt.query.client.Properties;
@@ -67,13 +72,6 @@ import java.util.List;
  */
 public class Transitions extends Effects {
 
-  // Used to check supported properties in the browser
-  protected static final Style divStyle = Document.get().createDivElement().getStyle();
-
-  public static final String prefix = browser.msie ? "ms" : browser.opera ? "o" : browser.mozilla ? "moz" : browser.webkit ? "webkit" : "";
-  private static final String transform = getVendorPropertyName("transform");
-  private static final String transformOrigin = getVendorPropertyName("transformOrigin");
-
   protected static final String transition = getVendorPropertyName("transition");
 
   // passing an invalid transition property in chrome, makes disable all transitions in the element
@@ -82,8 +80,6 @@ public class Transitions extends Effects {
   protected static final String transitionDelay = getVendorPropertyName("transitionDelay");
   protected static final String transitionEnd = browser.mozilla || browser.msie ? "transitionend" : (prefix + "TransitionEnd");
 
-  public static final boolean has3d = supportsTransform3d();
-
   public static final Class<Transitions> Transitions = GQuery.registerPlugin(
       Transitions.class, new Plugin<Transitions>() {
         public Transitions init(GQuery gq) {
@@ -91,40 +87,13 @@ public class Transitions extends Effects {
         }
       });
 
-  public static String getVendorPropertyName(String prop) {
-    assert divStyle != null;
-    // we prefer vendor specific names by default
-    String vendorProp =  JsUtils.camelize("-" + prefix + "-" + prop);
-    if (JsUtils.hasProperty(divStyle, vendorProp)) {
-      return vendorProp;
-    }
-    if (JsUtils.hasProperty(divStyle, prop)) {
-      return prop;
-    }
-    String camelProp = JsUtils.camelize(prop);
-    if (JsUtils.hasProperty(divStyle, camelProp)) {
-      return camelProp;
-    }
-    return null;
-  }
-
   private static String property(String prop) {
-    if (Transform.isTransform(prop)) {
+    if (isTransform(prop)) {
       return transform;
     }
     return prop.replaceFirst("^(margin|padding).+$", "$1");
   }
 
-  private static boolean supportsTransform3d() {
-    if (transform == null) {
-      return false;
-    }
-    String rotate = "rotateY(1deg)";
-    divStyle.setProperty(transform, rotate);
-    rotate = divStyle.getProperty(transform);
-    return rotate != null && !rotate.isEmpty();
-  }
-
   protected Transitions(GQuery gq) {
     super(gq);
   }
@@ -132,13 +101,13 @@ public class Transitions extends Effects {
   @Override
   public String css(String prop, boolean force) {
     if ("transform".equals(prop)) {
-      return isEmpty() ? "" : Transform.getInstance(get(0), null).toString();
+      return isEmpty() ? "" : getInstance(get(0), null).toString();
     } else if ("transformOrigin".equals(prop)) {
       return super.css(transformOrigin, force);
     } else if ("transition".equals(prop)) {
       return super.css(transition, force);
-    } else if (Transform.isTransform(prop)) {
-      return isEmpty() ? "" : Transform.getInstance(get(0), null).get(prop);
+    } else if (isTransform(prop)) {
+      return isEmpty() ? "" : getInstance(get(0), null).get(prop);
     } else {
       return super.css(prop, force);
     }
@@ -148,16 +117,16 @@ public class Transitions extends Effects {
   public Transitions css(String prop, String value) {
     if ("transform".equals(prop)) {
       for (Element e : elements()) {
-        Transform t = Transform.getInstance(e, value);
+        Transform t = getInstance(e, value);
         getStyleImpl().setStyleProperty(e, transform, t.toString());
       }
     } else if ("transformOrigin".equals(prop)) {
       super.css(transformOrigin, value);
     } else if ("transition".equals(prop)) {
       super.css(transition, value);
-    } else if (Transform.isTransform(prop)) {
+    } else if (isTransform(prop)) {
       for (Element e : elements()) {
-        Transform t = Transform.getInstance(e, null);
+        Transform t = getInstance(e, null);
         t.setFromString(prop, value);
         getStyleImpl().setStyleProperty(e, transform, t.toString());
       }
diff --git a/gwtquery-core/src/test/java/com/google/gwt/query/client/GQueryCoreTestGwt.java b/gwtquery-core/src/test/java/com/google/gwt/query/client/GQueryCoreTestGwt.java
index 36d38b70..4df6b85c 100644
--- a/gwtquery-core/src/test/java/com/google/gwt/query/client/GQueryCoreTestGwt.java
+++ b/gwtquery-core/src/test/java/com/google/gwt/query/client/GQueryCoreTestGwt.java
@@ -17,11 +17,9 @@ package com.google.gwt.query.client;
 
 import static com.google.gwt.query.client.GQuery.$;
 import static com.google.gwt.query.client.GQuery.$$;
-import static com.google.gwt.query.client.GQuery.console;
 import static com.google.gwt.query.client.GQuery.document;
 import static com.google.gwt.query.client.GQuery.window;
 
-import com.google.gwt.core.client.js.JsType;
 import com.google.gwt.dom.client.Document;
 import com.google.gwt.dom.client.Element;
 import com.google.gwt.dom.client.InputElement;
@@ -1032,11 +1030,19 @@ public class GQueryCoreTestGwt extends GWTTestCase {
   }
 
   public void testUtilsCallFunc() {
+    assertTrue(JsUtils.hasProperty(window, "document.body.style.background"));
+    assertTrue(JsUtils.hasProperty(window, "document.createElement"));
+    assertFalse(JsUtils.hasProperty(window, "document.body.style.foo"));
+
     Element e = JsUtils.runJavascriptFunction(document, "createElement", "div");
     assertNotNull(e);
     assertEquals(e.getTagName().toLowerCase(), "div");
 
-    e = JsUtils.runJavascriptFunction(document, "foo", "bar", 2, true);
+    e = JsUtils.jsni("document.createElement", "span");
+    assertNotNull(e);
+    assertEquals(e.getTagName().toLowerCase(), "span");
+
+    e = JsUtils.jsni("document.foo.bar");
     assertNull(e);
   }
 
diff --git a/gwtquery-core/src/test/java/com/google/gwt/query/client/GQueryEffectsTestGwt.java b/gwtquery-core/src/test/java/com/google/gwt/query/client/GQueryEffectsTestGwt.java
index 9658e98b..0b210802 100644
--- a/gwtquery-core/src/test/java/com/google/gwt/query/client/GQueryEffectsTestGwt.java
+++ b/gwtquery-core/src/test/java/com/google/gwt/query/client/GQueryEffectsTestGwt.java
@@ -31,6 +31,7 @@ import com.google.gwt.query.client.plugins.effects.Fx.ColorFx;
 import com.google.gwt.query.client.plugins.effects.Fx.TransitFx;
 import com.google.gwt.query.client.plugins.effects.PropertiesAnimation;
 import com.google.gwt.query.client.plugins.effects.PropertiesAnimation.EasingCurve;
+import com.google.gwt.query.client.plugins.effects.Transform;
 import com.google.gwt.query.client.plugins.effects.TransitionsAnimation;
 import com.google.gwt.query.client.plugins.effects.TransitionsAnimation.TransitionsClipAnimation;
 import com.google.gwt.user.client.Timer;
@@ -39,6 +40,9 @@ import com.google.gwt.user.client.ui.Label;
 import com.google.gwt.user.client.ui.RootPanel;
 import com.google.gwt.user.client.ui.Widget;
 
+import java.util.Arrays;
+import java.util.List;
+
 /**
  * Test class for testing gwtquery effects plugin api.
  */
@@ -312,6 +316,35 @@ public class GQueryEffectsTestGwt extends GWTTestCase {
             .toString());
   }
 
+  public void testTransformParser() {
+    Transform.has3d = true;
+    Transform t = new Transform("scaleZ(0.5) rotateZ(90deg) skewX(4) rotateY(45) scale(1, 1) x(10) y(12) z(14) matrix(1, 2,3 ,4)");
+    List<String> vals = Arrays.asList(t.toString().split(" "));
+    // scale(1,1) matrix(1,2,3,4) rotateZ(90deg) translateZ(14px) rotateY(45deg) translateY(12px) skewX(4deg) translateX(10px) scaleZ(0.5)
+    assertEquals(9, vals.size());
+    assertTrue(vals.contains("scaleZ(0.5)"));
+    assertTrue(vals.contains("rotateZ(90deg)"));
+    assertTrue(vals.contains("rotateY(45deg)"));
+    assertTrue(vals.contains("skewX(4deg)"));
+    assertTrue(vals.contains("rotateY(45deg)"));
+    assertTrue(vals.contains("scale(1,1)"));
+    assertTrue(vals.contains("translateX(10px)"));
+    assertTrue(vals.contains("translateY(12px)"));
+    assertTrue(vals.contains("translateZ(14px)"));
+    assertTrue(vals.contains("matrix(1,2,3,4)"));
+
+    Transform.has3d = false;
+    t = new Transform("scaleZ(0.5) rotateZ(90deg) skewX(4) rotateY(45) scale(1, 1) x(10) y(12) z(14) matrix(1, 2,3 ,4)");
+    vals = Arrays.asList(t.toString().split(" "));
+    // scale(1,1) matrix(1,2,3,4) translateY(12px) skewX(4deg) translateX(10px)
+    assertEquals(5, vals.size());
+    assertTrue(vals.contains("scale(1,1)"));
+    assertTrue(vals.contains("skewX(4deg)"));
+    assertTrue(vals.contains("translateX(10px)"));
+    assertTrue(vals.contains("translateY(12px)"));
+    assertTrue(vals.contains("matrix(1,2,3,4)"));
+  }
+
   public void testTransitionsAnimation() {
     final GQuery m = $("<div style='top: 10px; width:50px'>foo</div>").appendTo(e);