[gwt-contrib] Change in gwt[master]: Fix non-final field initializers running before the super cstr.

2013-05-30 Thread Roberto Lublinerman

Roberto Lublinerman has posted comments on this change.

Change subject: Fix non-final field initializers running before the super  
cstr.

..


Patch Set 8: Code-Review+2

--
To view, visit https://gwt-review.googlesource.com/3030
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4c8ed0cd718a2188b33cc290fec6071c89be7918
Gerrit-PatchSet: 8
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Stephen Haberman stephen.haber...@gmail.com
Gerrit-Reviewer: Leeroy Jenkins jenk...@gwtproject.org
Gerrit-Reviewer: Matthew Dempsky mdemp...@google.com
Gerrit-Reviewer: Ray Cromwell cromwell...@google.com
Gerrit-Reviewer: Roberto Lublinerman rlu...@google.com
Gerrit-Reviewer: Stephen Haberman stephen.haber...@gmail.com
Gerrit-HasComments: No

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Add hasClassName method in com.google.gwt.dom.client.Element

2013-05-30 Thread Goktug Gokdogan

Goktug Gokdogan has posted comments on this change.

Change subject: Add hasClassName method in com.google.gwt.dom.client.Element
..


Patch Set 3:

Hi Colin. I understand your concerns.

However, I like to remind that we are only promising API compatibility in  
maintenance releases like (2.5.1 to 2.5.2). Waiting major releases for  
these kind of patches would mean that delays up to 6 months which is not  
very practical and acceptable pace - especially for Google's use. Being  
said that, we are also trying to avoid changes that would cause too much  
refactoring to do an upgrade. In this case, the fix is pretty  
straightforward for most end user usages. If this one is a real big trouble  
for you, then we can wait for another 4-5 months but I would not prefer  
that =)


For reducing the pain in the future; I don't expect this class to grow much  
but I wonder if it is possible for you to compose instead of extend this  
kind of classes? If you can't, feel free to propose other changes that you  
expect to see in this base class instead of adding it your own. For  
example, I'm also planning to add toggleClassName.


--
To view, visit https://gwt-review.googlesource.com/3070
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia09567b8c58cac02f8126c33ef169b26def3d19c
Gerrit-PatchSet: 3
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Andrey Korzhevskiy a.korzhevs...@gmail.com
Gerrit-Reviewer: Andrey Korzhevskiy a.korzhevs...@gmail.com
Gerrit-Reviewer: Colin Alworth niloc...@gmail.com
Gerrit-Reviewer: Daniel Kurka danku...@google.com
Gerrit-Reviewer: Goktug Gokdogan gok...@google.com
Gerrit-HasComments: No

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Add hasClassName method in com.google.gwt.dom.client.Element

2013-05-30 Thread Thomas Broyer

Thomas Broyer has posted comments on this change.

Change subject: Add hasClassName method in com.google.gwt.dom.client.Element
..


Patch Set 3: Code-Review+1

--
To view, visit https://gwt-review.googlesource.com/3070
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia09567b8c58cac02f8126c33ef169b26def3d19c
Gerrit-PatchSet: 3
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Andrey Korzhevskiy a.korzhevs...@gmail.com
Gerrit-Reviewer: Andrey Korzhevskiy a.korzhevs...@gmail.com
Gerrit-Reviewer: Colin Alworth niloc...@gmail.com
Gerrit-Reviewer: Daniel Kurka danku...@google.com
Gerrit-Reviewer: Goktug Gokdogan gok...@google.com
Gerrit-Reviewer: Thomas Broyer t.bro...@gmail.com
Gerrit-HasComments: No

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Use JSON.parse() instead of eval() to deserialize rpc callba...

2013-05-30 Thread Artur Signell

Artur Signell has posted comments on this change.

Change subject: Use JSON.parse() instead of eval() to deserialize rpc  
callback payload

..


Patch Set 4:

Would it be a feasible solution to introduce protocol version 8 today  
(which is valid JSON) but keep the default version at 7 until we drop  
support for IE6/IE7? This would disconnect this issue from the IE6/IE7  
discussion.


By making it possible to change the server to use version 8 for  
communicating with the client (through e.g. a system property) any user can  
today fix the memory leak issue in IE9 (provided they do not need IE6/IE7  
support) and all other users will get the fix automatically once we drop  
old IE support and bump the default version.


--
To view, visit https://gwt-review.googlesource.com/2900
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6062180397f5fabed1dd5f08140c2bd43a19fa9f
Gerrit-PatchSet: 4
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: John Ahlroos j...@vaadin.com
Gerrit-Reviewer: Artur Signell ar...@vaadin.com
Gerrit-Reviewer: Brian Slesinsky skybr...@google.com
Gerrit-Reviewer: Colin Alworth niloc...@gmail.com
Gerrit-Reviewer: John Ahlroos j...@vaadin.com
Gerrit-Reviewer: Leeroy Jenkins jenk...@gwtproject.org
Gerrit-Reviewer: Matthew Dempsky mdemp...@google.com
Gerrit-Reviewer: Thomas Broyer t.bro...@gmail.com
Gerrit-HasComments: No

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Ensures integer pixel values and adds getters for subpixel v...

2013-05-30 Thread John Ahlroos

John Ahlroos has uploaded a new change for review.

  https://gwt-review.googlesource.com/3090


Change subject: Ensures integer pixel values and adds getters for subpixel  
values

..

Ensures integer pixel values and adds getters for subpixel values

Newer browsers are returning pixel values as doubles instead of
integers especially when using zooming. This is causing problems where
existing code is assuming integer values are returned and getting
doubles instead.

To fix this this patch introduces subpixel variants of most DOM
methods dealing with pixels and makes sure the existing methods
are returning only valid integers by 'x|0' them.

Issue: http://code.google.com/p/google-web-toolkit/issues/detail?id=6130
Change-Id: I27e98f0ad3c1c236b2d85d24f197719fa4cd352a
Rietveld: http://gwt-code-reviews.appspot.com/1815803
---
M user/src/com/google/gwt/dom/client/DOMImpl.java
M user/src/com/google/gwt/dom/client/DOMImplIE6.java
M user/src/com/google/gwt/dom/client/DOMImplIE8.java
M user/src/com/google/gwt/dom/client/DOMImplIE9.java
M user/src/com/google/gwt/dom/client/DOMImplMozilla.java
M user/src/com/google/gwt/dom/client/DOMImplOpera.java
M user/src/com/google/gwt/dom/client/DOMImplStandard.java
M user/src/com/google/gwt/dom/client/DOMImplStandardBase.java
M user/src/com/google/gwt/dom/client/DOMImplTrident.java
M user/src/com/google/gwt/dom/client/Element.java
M user/src/com/google/gwt/dom/client/NativeEvent.java
11 files changed, 377 insertions(+), 150 deletions(-)



diff --git a/user/src/com/google/gwt/dom/client/DOMImpl.java  
b/user/src/com/google/gwt/dom/client/DOMImpl.java

index 56f1450..ddbc8aa 100644
--- a/user/src/com/google/gwt/dom/client/DOMImpl.java
+++ b/user/src/com/google/gwt/dom/client/DOMImpl.java
@@ -68,8 +68,8 @@
   int charCode);

   public abstract NativeEvent createMouseEvent(Document doc, String type,
-  boolean canBubble, boolean cancelable, int detail, int screenX,
-  int screenY, int clientX, int clientY, boolean ctrlKey, boolean  
altKey,

+  boolean canBubble, boolean cancelable, int detail, double screenX,
+  double screenY, double clientX, double clientY, boolean ctrlKey,  
boolean altKey,
   boolean shiftKey, boolean metaKey, int button, Element  
relatedTarget);


   public ScriptElement createScriptElement(Document doc, String source) {
@@ -110,13 +110,13 @@

   public abstract int eventGetCharCode(NativeEvent evt);

-  public native int eventGetClientX(NativeEvent evt) /*-{
-return evt.clientX || 0;
-  }-*/;
+  public final int eventGetClientX(NativeEvent evt) {
+return (int) eventGetSubpixelClientX(evt) | 0;
+  }

-  public native int eventGetClientY(NativeEvent evt) /*-{
-return evt.clientY || 0;
-  }-*/;
+  public final int eventGetClientY(NativeEvent evt) {
+return (int) eventGetSubpixelClientY(evt) | 0;
+  }

   public native boolean eventGetCtrlKey(NativeEvent evt) /*-{
 return !!evt.ctrlKey;
@@ -146,16 +146,32 @@
 return evt.scale;
   }-*/;

-  public native int eventGetScreenX(NativeEvent evt) /*-{
-return evt.screenX || 0;
-  }-*/;
+  public final int eventGetScreenX(NativeEvent evt) {
+return (int) eventGetSubpixelScreenX(evt) | 0;
+  }

-  public native int eventGetScreenY(NativeEvent evt) /*-{
-return evt.screenY || 0;
-  }-*/;
+  public final int eventGetScreenY(NativeEvent evt) {
+return (int) eventGetSubpixelScreenY(evt) | 0;
+  }

   public native boolean eventGetShiftKey(NativeEvent evt) /*-{
 return !!evt.shiftKey;
+  }-*/;
+
+  public native double eventGetSubpixelClientX(NativeEvent evt) /*-{
+return evt.clientX || 0;
+  }-*/;
+
+  public native double eventGetSubpixelClientY(NativeEvent evt) /*-{
+return evt.clientY || 0;
+  }-*/;
+
+  public native double eventGetSubpixelScreenX(NativeEvent evt) /*-{
+return evt.screenX || 0;
+  }-*/;
+
+  public native double eventGetSubpixelScreenY(NativeEvent evt) /*-{
+return evt.screenY || 0;
   }-*/;

   public abstract EventTarget eventGetTarget(NativeEvent evt);
@@ -176,47 +192,25 @@

   public abstract String eventToString(NativeEvent evt);

-  public native int getAbsoluteLeft(Element elem) /*-{
-var left = 0;
-var curr = elem;
-// This intentionally excludes body which has a null offsetParent.
-while (curr.offsetParent) {
-  left -= curr.scrollLeft;
-  curr = curr.parentNode;
-}
-while (elem) {
-  left += elem.offsetLeft;
-  elem = elem.offsetParent;
-}
-return left;
-  }-*/;
+  public final int getAbsoluteLeft(Element elem) {
+return (int) getSubpixelAbsoluteLeft(elem) | 0;
+  }

-  public native int getAbsoluteTop(Element elem) /*-{
-var top = 0;
-var curr = elem;
-// This intentionally excludes body which has a null offsetParent.
-while (curr.offsetParent) {
-  top -= curr.scrollTop;
-  curr = curr.parentNode;
-}
-while (elem) {
-  top += elem.offsetTop;
-  elem 

[gwt-contrib] Change in gwt[master]: Workaround for resolving mouse button on mouse move in Firefox

2013-05-30 Thread John Ahlroos

John Ahlroos has posted comments on this change.

Change subject: Workaround for resolving mouse button on mouse move in  
Firefox

..


Patch Set 3:

Is there something still missing for this to get it accepted?

--
To view, visit https://gwt-review.googlesource.com/2330
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib73997af56ce0e7da5b41814a7ac2b208ab022aa
Gerrit-PatchSet: 3
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: John Ahlroos j...@vaadin.com
Gerrit-Reviewer: Goktug Gokdogan gok...@google.com
Gerrit-Reviewer: John Ahlroos j...@vaadin.com
Gerrit-Reviewer: Matthew Dempsky mdemp...@google.com
Gerrit-Reviewer: Matthew Dempsky mdemp...@gwtproject.org
Gerrit-HasComments: No

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Set KeyboardSelectedRow with relative index in AbstractHasDa...

2013-05-30 Thread Thomas Broyer

Thomas Broyer has posted comments on this change.

Change subject: Set KeyboardSelectedRow with relative index in  
AbstractHasData focus event preview

..


Patch Set 2: Code-Review+2

--
To view, visit https://gwt-review.googlesource.com/3012
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I319b10f109914237d1a26ea7e0e401bfab5a691b
Gerrit-PatchSet: 2
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Isaiah Billingsley iza.billings...@gmail.com
Gerrit-Reviewer: Thomas Broyer t.bro...@gmail.com
Gerrit-HasComments: No

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Re: SuggestBox causes native events to fire twice (3533) (issue1785803)

2013-05-30 Thread tuckerpmt

If you can finish it up that would be great.

I haven't really had time to figure out the new system.  With the old
system I could generate patch files from any of the many computers that
I work on, email it to myself and submit the patch from anywhere.  I
can't really do that anymore...

On 2013/05/29 20:41:32, goktug wrote:

On 2012/09/05 15:44:32, Patrick Tucker wrote:
 On 2012/07/19 20:48:12, tbroyer wrote:

 Can this patch be committed?



Can you move this patch to gerrit or would you prefer me to do that

for you?

http://gwt-code-reviews.appspot.com/1785803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Allow line breaks and other whitespace in jsni method refere...

2013-05-30 Thread Roberto Lublinerman

Roberto Lublinerman has uploaded a new change for review.

  https://gwt-review.googlesource.com/3100


Change subject: Allow line breaks and other whitespace in jsni method  
references.

..

Allow line breaks and other whitespace in jsni method references.

This allows for whitespace in the param list within jsni method references  
so

long lines can be broken up.

Adds several test cases for JSNI references.

Original Author: Kelly Campbell kelly.a.campb...@gmail.com

Change-Id: Iecacf3b8be1e512a5fdf3132245091b4ebea9c68
---
M dev/core/src/com/google/gwt/dev/js/rhino/TokenStream.java
M dev/core/test/com/google/gwt/dev/js/TokenStreamTest.java
2 files changed, 43 insertions(+), 11 deletions(-)



diff --git a/dev/core/src/com/google/gwt/dev/js/rhino/TokenStream.java  
b/dev/core/src/com/google/gwt/dev/js/rhino/TokenStream.java

index 934eabb..a677b31 100644
--- a/dev/core/src/com/google/gwt/dev/js/rhino/TokenStream.java
+++ b/dev/core/src/com/google/gwt/dev/js/rhino/TokenStream.java
@@ -1343,6 +1343,15 @@
 }
 }

+private void skipWhitespace() throws IOException {
+  int tmp;
+  do {
+tmp = in.read();
+  } while (isJSSpace(tmp) || tmp == '\n');
+  // Reposition back to first non whitespace char.
+  in.unread();
+}
+
 private int jsniMatchReference() throws IOException {

   // First, read the type name whose member is being accessed.
@@ -1359,9 +1368,11 @@
   return ERROR;
   }
   addToString(c);
-
+
+  // Skip whitespace starting after ::.
+  skipWhitespace();
+
   // Finish by reading the field or method signature.
-  //
   if (!jsniMatchMethodSignatureOrFieldName()) {
 return ERROR;
   }
@@ -1373,7 +1384,9 @@
 private boolean jsniMatchParamListSignature() throws IOException {
   // Assume the opening '(' has already been read.
   // Read param type signatures until we see a closing ')'.
-  //
+
+  skipWhitespace();
+
   // First check for the special case of * as the parameter list,  
indicating

   // a wildcard
   if (in.peek() == '*') {
@@ -1384,20 +1397,24 @@
 addToString(in.read());
 return true;
   }
-
+
   // Otherwise, loop through reading one param type at a time
   do {
+// Skip whitespace between parameters.
+skipWhitespace();
+
 int c = in.read();
+
 if (c == ')') {
   // Finished successfully.
   //
   addToString(c);
   return true;
 }
-
+
 in.unread();
   } while (jsniMatchParamTypeSignature());
-
+
   // If we made it here, we can assume that there was an invalid type
   // signature that was already reported and that the offending char
   // was already unread.
@@ -1443,7 +1460,8 @@

 private boolean jsniMatchMethodSignatureOrFieldName() throws  
IOException {

   int c = in.read();
-
+
+
   // We must see an ident start here.
   //
   if (!Character.isJavaIdentifierStart((char)c)) {
diff --git a/dev/core/test/com/google/gwt/dev/js/TokenStreamTest.java  
b/dev/core/test/com/google/gwt/dev/js/TokenStreamTest.java

index 9f8db11..dabb98d 100644
--- a/dev/core/test/com/google/gwt/dev/js/TokenStreamTest.java
+++ b/dev/core/test/com/google/gwt/dev/js/TokenStreamTest.java
@@ -1,12 +1,12 @@
 /*
  * Copyright 2009 Google Inc.
- *
+ *
  * Licensed under the Apache License, Version 2.0 (the License); you may  
not
  * use this file except in compliance with the License. You may obtain a  
copy of

  * the License at
- *
+ *
  * http://www.apache.org/licenses/LICENSE-2.0
- *
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an AS IS BASIS,  
WITHOUT

  * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
@@ -55,6 +55,7 @@
 assertGoodJsni(@org.group.Foo::bar(I));
 assertGoodJsni(@org.group.Foo::bar(IJ));
 assertGoodJsni(@org.group.Foo::bar(Lorg/group/Foo;));
+assertGoodJsni(@org.group.Foo::bar(Lorg/group/Foo;Lorg/group/Bar;));
 assertGoodJsni(@org.group.Foo::bar([I));
 // The following is currently tolerated
 // assertBadJsni(@org.group.Foo::bar(Lorg/group/Foo));
@@ -64,6 +65,18 @@
 // Method refs with * as the parameter list
 assertGoodJsni(@org.group.Foo::bar(*));
 assertBadJsni(@org.group.Foo::bar(*);
+
+// Refs that span lines
+assertGoodJsni(@org.group.Foo::bar(\nLorg/group/Foo;));
+assertGoodJsni(@org.group.Foo::bar(\nLorg/group/Foo;\n));
+assertGoodJsni(@org.group.Foo::bar(\n[I\n));
+assertGoodJsni(@org.group.Foo::bar(\nZ\nZ\n));
+assertGoodJsni(@org.group.Foo::bar(\nLorg/group/Foo;\nZ));
+assertGoodJsni(@org.group.Foo::\nbar());
+
+
+assertBadJsni(@org.group.Foo::bar(\nLorg/group/Foo;,\nZ));
+assertBadJsni(@org.group.Foo::bar(\nLorg/group/Foo,\nZ));

 // bad references
  

[gwt-contrib] Change in gwt[master]: Allow line breaks and other whitespace in jsni method refere...

2013-05-30 Thread Thomas Broyer

Thomas Broyer has posted comments on this change.

Change subject: Allow line breaks and other whitespace in jsni method  
references.

..


Patch Set 1: Code-Review+2

(1 comment)


File dev/core/test/com/google/gwt/dev/js/TokenStreamTest.java
Line 78: assertBadJsni(@org.group.Foo::bar(\nLorg/group/Foo;,\nZ));
Should we add something with a line-break in the middle of a L? e.g.

 assertBadJsni(@org.group.Foo::bar(Lorg/group/\nFoo;));


--
To view, visit https://gwt-review.googlesource.com/3100
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iecacf3b8be1e512a5fdf3132245091b4ebea9c68
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Roberto Lublinerman rlu...@google.com
Gerrit-Reviewer: Daniel Kurka danku...@google.com
Gerrit-Reviewer: Leeroy Jenkins jenk...@gwtproject.org
Gerrit-Reviewer: Thomas Broyer t.bro...@gmail.com
Gerrit-HasComments: Yes

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Allow line breaks and other whitespace in jsni method refere...

2013-05-30 Thread Roberto Lublinerman

Roberto Lublinerman has submitted this change and it was merged.

Change subject: Allow line breaks and other whitespace in jsni method  
references.

..


Allow line breaks and other whitespace in jsni method references.

This allows for whitespace in the param list within jsni method references  
so

long lines can be broken up.

Adds several test cases for JSNI references.

Original Author: Kelly Campbell kelly.a.campb...@gmail.com

Change-Id: Iecacf3b8be1e512a5fdf3132245091b4ebea9c68
---
M dev/core/src/com/google/gwt/dev/js/rhino/TokenStream.java
M dev/core/test/com/google/gwt/dev/js/TokenStreamTest.java
2 files changed, 43 insertions(+), 11 deletions(-)

Approvals:
  Leeroy Jenkins: Verified
  Thomas Broyer: Looks good to me, approved



diff --git a/dev/core/src/com/google/gwt/dev/js/rhino/TokenStream.java  
b/dev/core/src/com/google/gwt/dev/js/rhino/TokenStream.java

index 934eabb..a677b31 100644
--- a/dev/core/src/com/google/gwt/dev/js/rhino/TokenStream.java
+++ b/dev/core/src/com/google/gwt/dev/js/rhino/TokenStream.java
@@ -1343,6 +1343,15 @@
 }
 }

+private void skipWhitespace() throws IOException {
+  int tmp;
+  do {
+tmp = in.read();
+  } while (isJSSpace(tmp) || tmp == '\n');
+  // Reposition back to first non whitespace char.
+  in.unread();
+}
+
 private int jsniMatchReference() throws IOException {

   // First, read the type name whose member is being accessed.
@@ -1359,9 +1368,11 @@
   return ERROR;
   }
   addToString(c);
-
+
+  // Skip whitespace starting after ::.
+  skipWhitespace();
+
   // Finish by reading the field or method signature.
-  //
   if (!jsniMatchMethodSignatureOrFieldName()) {
 return ERROR;
   }
@@ -1373,7 +1384,9 @@
 private boolean jsniMatchParamListSignature() throws IOException {
   // Assume the opening '(' has already been read.
   // Read param type signatures until we see a closing ')'.
-  //
+
+  skipWhitespace();
+
   // First check for the special case of * as the parameter list,  
indicating

   // a wildcard
   if (in.peek() == '*') {
@@ -1384,20 +1397,24 @@
 addToString(in.read());
 return true;
   }
-
+
   // Otherwise, loop through reading one param type at a time
   do {
+// Skip whitespace between parameters.
+skipWhitespace();
+
 int c = in.read();
+
 if (c == ')') {
   // Finished successfully.
   //
   addToString(c);
   return true;
 }
-
+
 in.unread();
   } while (jsniMatchParamTypeSignature());
-
+
   // If we made it here, we can assume that there was an invalid type
   // signature that was already reported and that the offending char
   // was already unread.
@@ -1443,7 +1460,8 @@

 private boolean jsniMatchMethodSignatureOrFieldName() throws  
IOException {

   int c = in.read();
-
+
+
   // We must see an ident start here.
   //
   if (!Character.isJavaIdentifierStart((char)c)) {
diff --git a/dev/core/test/com/google/gwt/dev/js/TokenStreamTest.java  
b/dev/core/test/com/google/gwt/dev/js/TokenStreamTest.java

index 9f8db11..dabb98d 100644
--- a/dev/core/test/com/google/gwt/dev/js/TokenStreamTest.java
+++ b/dev/core/test/com/google/gwt/dev/js/TokenStreamTest.java
@@ -1,12 +1,12 @@
 /*
  * Copyright 2009 Google Inc.
- *
+ *
  * Licensed under the Apache License, Version 2.0 (the License); you may  
not
  * use this file except in compliance with the License. You may obtain a  
copy of

  * the License at
- *
+ *
  * http://www.apache.org/licenses/LICENSE-2.0
- *
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an AS IS BASIS,  
WITHOUT

  * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
@@ -55,6 +55,7 @@
 assertGoodJsni(@org.group.Foo::bar(I));
 assertGoodJsni(@org.group.Foo::bar(IJ));
 assertGoodJsni(@org.group.Foo::bar(Lorg/group/Foo;));
+assertGoodJsni(@org.group.Foo::bar(Lorg/group/Foo;Lorg/group/Bar;));
 assertGoodJsni(@org.group.Foo::bar([I));
 // The following is currently tolerated
 // assertBadJsni(@org.group.Foo::bar(Lorg/group/Foo));
@@ -64,6 +65,18 @@
 // Method refs with * as the parameter list
 assertGoodJsni(@org.group.Foo::bar(*));
 assertBadJsni(@org.group.Foo::bar(*);
+
+// Refs that span lines
+assertGoodJsni(@org.group.Foo::bar(\nLorg/group/Foo;));
+assertGoodJsni(@org.group.Foo::bar(\nLorg/group/Foo;\n));
+assertGoodJsni(@org.group.Foo::bar(\n[I\n));
+assertGoodJsni(@org.group.Foo::bar(\nZ\nZ\n));
+assertGoodJsni(@org.group.Foo::bar(\nLorg/group/Foo;\nZ));
+assertGoodJsni(@org.group.Foo::\nbar());
+
+
+assertBadJsni(@org.group.Foo::bar(\nLorg/group/Foo;,\nZ));
+

[gwt-contrib] Change in gwt[master]: Allow line breaks and other whitespace in jsni method refere...

2013-05-30 Thread Roberto Lublinerman

Roberto Lublinerman has abandoned this change.

Change subject: Allow line breaks and other whitespace in jsni method  
references.

..


Abandoned

Thanks, Kelly. I have submitted it as change  
Iecacf3b8be1e512a5fdf3132245091b4ebea9c68


--
To view, visit https://gwt-review.googlesource.com/2640
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: abandon
Gerrit-Change-Id: I64426b03f4cde2c84c2faf27a2e38aba4720f401
Gerrit-PatchSet: 2
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Kelly Campbell kelly.a.campb...@gmail.com
Gerrit-Reviewer: Kelly Campbell kelly.a.campb...@gmail.com
Gerrit-Reviewer: Leeroy Jenkins jenk...@gwtproject.org
Gerrit-Reviewer: Matthew Dempsky mdemp...@gwtproject.org
Gerrit-Reviewer: Roberto Lublinerman rlu...@gmail.com
Gerrit-Reviewer: Thomas Broyer t.bro...@gmail.com

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Fix non-final field initializers running before the super cstr.

2013-05-30 Thread Stephen Haberman

Stephen Haberman has submitted this change and it was merged.

Change subject: Fix non-final field initializers running before the super  
cstr.

..


Fix non-final field initializers running before the super cstr.

Previously, any field with an initializer would get assigned at
the top-level scope, before any cstrs had run.

However, this does not match the JVM behavior, which is that final fields  
behave

this way, but non-field fields have their type's default value assigned when
super cstrs run, and then only later in their cstr are assigned to the  
initializer.


Bug: issue 380
Change-Id: I4c8ed0cd718a2188b33cc290fec6071c89be7918
---
M dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
M user/test/com/google/gwt/dev/jjs/test/CompilerTest.java
A user/test/com/google/gwt/dev/jjs/test/FieldInitOrderBase.java
A user/test/com/google/gwt/dev/jjs/test/FieldInitOrderChild.java
4 files changed, 113 insertions(+), 10 deletions(-)

Approvals:
  Roberto Lublinerman: Looks good to me, approved
  Leeroy Jenkins: Verified



diff --git  
a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java  
b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java

index c0e1423..240f6a0 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
@@ -857,10 +857,13 @@
   JsNameRef localRef = (JsNameRef) pop(); // localRef

   JVariable target = x.getVariableRef().getTarget();
-  if (target instanceof JField  ((JField)  
target).getLiteralInitializer() != null) {

-// Will initialize at top scope; no need to double-initialize.
-push(null);
-return;
+  if (target instanceof JField) {
+JField field = (JField) target;
+if (field.getLiteralInitializer() != null  (field.isStatic() ||  
field.isFinal())) {

+  // Will initialize at top scope; no need to double-initialize.
+  push(null);
+  return;
+}
   }

   JsBinaryOperation binOp =
@@ -890,15 +893,15 @@
 @Override
 public void endVisit(JField x, Context ctx) {
   // if we need an initial value, create an assignment
-  if (x.getLiteralInitializer() != null) {
+  if (x.getLiteralInitializer() != null  (x.isFinal() ||  
x.isStatic())) {

 // setup the constant value
 accept(x.getLiteralInitializer());
-  } else if (!x.hasInitializer()  x.getEnclosingType() !=  
program.getTypeJavaLangObject()) {

-// setup a default value
-accept(x.getType().getDefaultValue());
-  } else {
-// the variable is setup during clinit, no need to initialize here
+  } else if (x.getEnclosingType() == program.getTypeJavaLangObject()) {
+// Special fields whose initialization is done somewhere else.
 push(null);
+  } else {
+// setup the default value, see Issue 380
+accept(x.getType().getDefaultValue());
   }
   JsExpression rhs = (JsExpression) pop();
   JsName name = names.get(x);
diff --git a/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java  
b/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java

index 4107144..812c6bb 100644
--- a/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java
+++ b/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java
@@ -787,6 +787,14 @@
 assertEquals(1, x);
   }

+  /** Ensure that only final fields are initializers when cstrs run, see  
issue 380. */

+  public void testFieldInitializationOrder() {
+ArrayListString seenValues = new ArrayListString();
+new FieldInitOrderChild(seenValues);
+assertEquals(i1=1,i2=0,i3=null,i4=null,i5=1,i6=1,i7=1,  
seenValues.get(0));

+assertEquals(i1=1,i2=1,i3=1,i4=2,i5=1,i6=2,i7=2, seenValues.get(1));
+  }
+
   public void testForStatement() {
 {
   int i;
diff --git a/user/test/com/google/gwt/dev/jjs/test/FieldInitOrderBase.java  
b/user/test/com/google/gwt/dev/jjs/test/FieldInitOrderBase.java

new file mode 100644
index 000..de1146b
--- /dev/null
+++ b/user/test/com/google/gwt/dev/jjs/test/FieldInitOrderBase.java
@@ -0,0 +1,33 @@
+/*
+ * Copyright 2013 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the License); you may  
not
+ * use this file except in compliance with the License. You may obtain a  
copy of

+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,  
WITHOUT

+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations  
under

+ * the License.
+ */
+package com.google.gwt.dev.jjs.test;
+
+import java.util.ArrayList;
+
+/**
+ * A superclass that invokes a method in its cstr, so that subclasses can  

Re: [gwt-contrib] Re: Removing support for old browsers

2013-05-30 Thread Brian Slesinsky
I would like soft permutations become a feature that we can have turned on
by default rather than one that you can theoretically use but is kind of
risky. Like with draft mode (which we just made the default at Google for
developers), I think there will be migration process where we fix bugs
before using it all the time.


On Wed, May 29, 2013 at 7:51 AM, Thomas Broyer t.bro...@gmail.com wrote:



 On Tuesday, May 28, 2013 8:12:44 PM UTC+2, Goktug Gokdogan wrote:

 Agreed. I think we need to think about the permutations and soft vs. hard
 for future releases. We started to have more and more newer vs older
 separations instead of Browser A ve Browser B separations, especially with
 the new-ish IEs.


 Maybe it no longer holds but when I first added support for
 requestAnimationFrame I used soft permutations instead of a runtime check
 and it was reverted as it broke projects inside Google:
 https://code.google.com/p/google-web-toolkit/source/detail?r=9986
 (in the end, soft permutations were ditched for other reasons:
 https://code.google.com/p/google-web-toolkit/source/detail?r=10257, and
 maybe the build failures were due to a bug in soft perms:
 https://code.google.com/p/google-web-toolkit/issues/detail?id=7409)

 --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors
 ---
 You received this message because you are subscribed to the Google Groups
 GWT Contributors group.
 To unsubscribe from this group and stop receiving emails from it, send an
 email to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
 For more options, visit https://groups.google.com/groups/opt_out.




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT 
Contributors group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Re: Removing support for old browsers

2013-05-30 Thread Jens
How would soft permutations work if you have browser version specific JS 
hacks/workarounds implemented during JJS compilation? If you have a 
 workaround for IE6 and a different one for IE7 and you can't mix both, 
then you are probably in trouble, aren't you? I don't know if that's just a 
theoretical question or if this case actually exists in todays JJS 
compiler. I was just wondering about it as I always thought that soft 
permutations only work well for deferred binding and hard permutations can 
have the potential to be compiled totally different by the compiler.


-- J.

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT 
Contributors group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Ensures integer pixel values and adds getters for subpixel v...

2013-05-30 Thread Goktug Gokdogan

Goktug Gokdogan has posted comments on this change.

Change subject: Ensures integer pixel values and adds getters for subpixel  
values

..


Patch Set 1:

What about exposing this APIs little bit different so that it introduces  
less clutter in the base JSO type and will less likely to break existing  
sub-types?


e.g.

 element.getSubpixel().getHeight()
 element.getSubpixel().getScrollHeight()
 etc.

--
To view, visit https://gwt-review.googlesource.com/3090
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I27e98f0ad3c1c236b2d85d24f197719fa4cd352a
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: John Ahlroos j...@vaadin.com
Gerrit-Reviewer: Artur Signell ar...@vaadin.com
Gerrit-Reviewer: Brian Slesinsky skybr...@google.com
Gerrit-Reviewer: Goktug Gokdogan gok...@google.com
Gerrit-Reviewer: Leeroy Jenkins jenk...@gwtproject.org
Gerrit-Reviewer: Thomas Broyer t.bro...@gmail.com
Gerrit-HasComments: No

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




Re: [gwt-contrib] Re: Removing support for old browsers

2013-05-30 Thread John A. Tamplin
On Thu, May 30, 2013 at 2:19 PM, Jens jens.nehlme...@gmail.com wrote:

 How would soft permutations work if you have browser version specific JS
 hacks/workarounds implemented during JJS compilation? If you have a
  workaround for IE6 and a different one for IE7 and you can't mix both,
 then you are probably in trouble, aren't you? I don't know if that's just a
 theoretical question or if this case actually exists in todays JJS
 compiler. I was just wondering about it as I always thought that soft
 permutations only work well for deferred binding and hard permutations can
 have the potential to be compiled totally different by the compiler.


Soft perms work exactly the way hard perms do, the only difference is there
both implementations are included and a runtime check instantiates the
right one.

For example, GWT.create(Foo.class) will normally get replaced with new
FooIE6() or new FooStandard() at compile time for corresponding
permutations.  Soft perms do something similar to this (it is actually much
more complicated for efficiency, but the end result is similar):

getProperty(user-agent).equals(ie6) ? new FooIE6() : new FooStandard()

If you have a workaround that is different between IE6 and IE7, you already
have a problem since those are in the same permutation, so you already have
to be doing a runtime check.

-- 
John A. Tamplin

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT 
Contributors group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Re: Fixes #8036, properly sorting of places in PlaceHistoryGeneratorContext (issue1895803)

2013-05-30 Thread larsaaslin

On 2013/05/29 00:33:29, goktug wrote:

On 2013/05/28 19:29:46, aaslin wrote:
 On 2013/04/08 23:51:51, goktug wrote:
  On 2013/04/04 14:06:13, tbroyer wrote:
   We just need someone with commit rights. Calling in Matthew (at

random),

who
   will delegate if needed.
 
  I'm not sure if this doesn't introduce any new problems and solves

all

 problems
  with the comparator. Can you reproduce the problem in
  MostToLeastDerivedPlaceTypeComparatorTest and additional test

cases?


 The test I added in PlaceHistoryMapperGeneratorTest.java is an

example of

 reproducing the problem since it will fail if executed with the

original

version
 of MostToLeastDerivedPlaceTypeComparator.java. I also added a new

patch set

that
 fixes MostToLeastDerivedPlaceTypeComparatorTest.java which, from

what I can

see,
 haven't been working properly before. Please let me now if there is

anything

 more I can do.



Do you mind moving your patch to http://gwt-review.googlesource.com

which is our newer

code review system?


Absolutely. Is there a way to migrate this issue or do I have to create
a new one there?


About the test: Why did you change the order in the previous test case

instead

of adding new one?


Because the test will fail with the changed comparator since places
are not order from most to least derived.


I think it would be best to see the new comparator working in
as many scenarios as possible.



Perhaps also we can simplify this further as we touch it.  From what I

can see,

the only reason we have a TreeMap is because of getPlaceTypes call. We

can keep

the places in a LinkedHashMap and change getPlaceTypes to

getSortedPlaceTypes

and do the sort there. With that change we will avoid keeping a sorted

list also

we will no longer need to be consistent with equals in a comparator

(i.e. you

can just do return o2.getFlattenedSupertypeHierarchy().size() -
o1.getFlattenedSupertypeHierarchy().size;).



Anyway this refactoring is optional, I can do it later.


I'll gladly give the refactoring a go. Is there any particular reason to
use a LinkedHashMap over just a HashMap?

http://gwt-code-reviews.appspot.com/1895803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Add hasClassName method in com.google.gwt.dom.client.Element

2013-05-30 Thread Andrey Korzhevskiy

Hello Colin Alworth, Thomas Broyer, Goktug Gokdogan,

I'd like you to reexamine a change.  Please visit

https://gwt-review.googlesource.com/3070

to look at the new patch set (#4).

Change subject: Add hasClassName method in com.google.gwt.dom.client.Element
..

Add hasClassName method in com.google.gwt.dom.client.Element

Fixes issue 7550

Change-Id: Ia09567b8c58cac02f8126c33ef169b26def3d19c
---
M user/src/com/google/gwt/dom/client/Element.java
M user/test/com/google/gwt/dom/client/ElementTest.java
2 files changed, 29 insertions(+), 9 deletions(-)


--
To view, visit https://gwt-review.googlesource.com/3070
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia09567b8c58cac02f8126c33ef169b26def3d19c
Gerrit-PatchSet: 4
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Andrey Korzhevskiy a.korzhevs...@gmail.com
Gerrit-Reviewer: Andrey Korzhevskiy a.korzhevs...@gmail.com
Gerrit-Reviewer: Colin Alworth niloc...@gmail.com
Gerrit-Reviewer: Daniel Kurka danku...@google.com
Gerrit-Reviewer: Goktug Gokdogan gok...@google.com
Gerrit-Reviewer: Thomas Broyer t.bro...@gmail.com

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Add hasClassName method in com.google.gwt.dom.client.Element

2013-05-30 Thread Andrey Korzhevskiy

Andrey Korzhevskiy has posted comments on this change.

Change subject: Add hasClassName method in com.google.gwt.dom.client.Element
..


Patch Set 4:

Uploaded patch set 4:
  I removed javadoc for trimClassName

--
To view, visit https://gwt-review.googlesource.com/3070
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia09567b8c58cac02f8126c33ef169b26def3d19c
Gerrit-PatchSet: 4
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Andrey Korzhevskiy a.korzhevs...@gmail.com
Gerrit-Reviewer: Andrey Korzhevskiy a.korzhevs...@gmail.com
Gerrit-Reviewer: Colin Alworth niloc...@gmail.com
Gerrit-Reviewer: Daniel Kurka danku...@google.com
Gerrit-Reviewer: Goktug Gokdogan gok...@google.com
Gerrit-Reviewer: Thomas Broyer t.bro...@gmail.com
Gerrit-HasComments: No

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Re: Trouble with gwt push (contributor agreement)

2013-05-30 Thread Matic Petek
Hi, I solve this problem by really visiting 
https://gwt-review.googlesource.com/#/settings/contact; and write 
additional information. 

Regards,
   Matic


On Friday, May 24, 2013 12:51:55 AM UTC+2, Matic Petek wrote:

 Hi,
   I update Books section on gwtproject.org and I would like to send 
 updates to Gerrit for review. I Sign Electronically cont. agreement (by 
 filling the form). Should I do something alse? Thank you.

 Regards,
Matic

 -- --
 git push origin HEAD:refs/for/master
 Username for 'https://gwt.googlesource.com': git-maticpetek.gmail.com
 Password for 'https://git-maticpetek.gmail@gwt.googlesource.com': 
 fatal: remote error: Individual contributor agreement requires current 
 contact information.

 Please review your contact information:

   https://gwt-review.googlesource.com/#/settings/contact
 -- --


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT 
Contributors group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Re: Fixes #8036, properly sorting of places in PlaceHistoryGeneratorContext (issue1895803)

2013-05-30 Thread goktug

On 2013/05/30 19:20:02, aaslin wrote:

On 2013/05/29 00:33:29, goktug wrote:
 On 2013/05/28 19:29:46, aaslin wrote:
  On 2013/04/08 23:51:51, goktug wrote:
   On 2013/04/04 14:06:13, tbroyer wrote:
We just need someone with commit rights. Calling in Matthew

(at random),

 who
will delegate if needed.
  
   I'm not sure if this doesn't introduce any new problems and

solves all

  problems
   with the comparator. Can you reproduce the problem in
   MostToLeastDerivedPlaceTypeComparatorTest and additional test

cases?

 
  The test I added in PlaceHistoryMapperGeneratorTest.java is an

example of

  reproducing the problem since it will fail if executed with the

original

 version
  of MostToLeastDerivedPlaceTypeComparator.java. I also added a new

patch set

 that
  fixes MostToLeastDerivedPlaceTypeComparatorTest.java which, from

what I can

 see,
  haven't been working properly before. Please let me now if there

is anything

  more I can do.

 Do you mind moving your patch to http://gwt-review.googlesource.com

which is

our newer
 code review system?



Absolutely. Is there a way to migrate this issue or do I have to

create a new

one there?


You need to create a new one there.



 About the test: Why did you change the order in the previous test

case instead

 of adding new one?



Because the test will fail with the changed comparator since places

are not

order from most to least derived.



Hmm, I didn't look at the test case closely before now I see what is
going on. Test is actually not included in the suite before. Can you add
it to PlaceSuite so it becomes part of test harness.


 I think it would be best to see the new comparator working in
 as many scenarios as possible.

 Perhaps also we can simplify this further as we touch it.  From what

I can

see,
 the only reason we have a TreeMap is because of getPlaceTypes call.

We can

keep
 the places in a LinkedHashMap and change getPlaceTypes to

getSortedPlaceTypes

 and do the sort there. With that change we will avoid keeping a

sorted list

also
 we will no longer need to be consistent with equals in a comparator

(i.e. you

 can just do return o2.getFlattenedSupertypeHierarchy().size() -
 o1.getFlattenedSupertypeHierarchy().size;).

 Anyway this refactoring is optional, I can do it later.



I'll gladly give the refactoring a go. Is there any particular reason

to use a

LinkedHashMap over just a HashMap?


To reduce randomness of behavior throughout different compilations.


http://gwt-code-reviews.appspot.com/1895803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




Re: [gwt-contrib] Re: Removing support for old browsers

2013-05-30 Thread Jens


 If you have a workaround that is different between IE6 and IE7, you 
 already have a problem since those are in the same permutation, so you 
 already have to be doing a runtime check.


My bad.. I meant IE6 and IE8 ..so two different hard permutations because 
thats what should be soft permutations in the future right?. And maybe add 
the opera permutation to this thought as well as this permutation will also 
become old in the near future with opera's switch to Blink/V8 and the 
fact that GWT only supports the latest version of Opera. 

If you remove the whole GWT.create() stuff from the equation, my thought 
was that the GWT compiler can still produces optimized JS code for a given 
browser (hard permutation) that is maybe incompatible with a different 
browser. There are a lot of AST visitors in the JavaToJavaScriptCompiler 
and if some of them only execute for IE6/7 and others for IE8 and even 
others for Opera this may cause trouble if you put all these browsers in a 
single soft permutation, wouldn't it?

-- J.

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT 
Contributors group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Inliner produces incorrect code if a parameter is a new arra...

2013-05-30 Thread Ray Cromwell

Ray Cromwell has posted comments on this change.

Change subject: Inliner produces incorrect code if a parameter is a new  
array expression.

..


Patch Set 2: Code-Review+2

Is this overly conservative? If there is only one reference to such a  
parameter, than it would be safe to inline?


--
To view, visit https://gwt-review.googlesource.com/3040
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic083ac15d7d84ab4728c441fd590977cd684a87a
Gerrit-PatchSet: 2
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Roberto Lublinerman rlu...@google.com
Gerrit-Reviewer: Leeroy Jenkins jenk...@gwtproject.org
Gerrit-Reviewer: Matthew Dempsky mdemp...@google.com
Gerrit-Reviewer: Ray Cromwell cromwell...@google.com
Gerrit-HasComments: No

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Re: Removing support for old browsers

2013-05-30 Thread Ray Cromwell
One reason to have a permutation for blink is if we start doing VM specific
code gen (V8 vs Nitro vs *Monkey).  Another is blink performance around
layout might require substantially different workarounds on Mobile Safari.



On Mon, May 27, 2013 at 11:42 AM, Matthew Dempsky mdemp...@google.comwrote:

 [-gwt-steering, +gwt-contrib]

 On Mon, May 27, 2013 at 9:32 AM, Daniel Kurka danku...@google.com wrote:

 We also need the permutations for blink.


 Are there technical reasons for why we need to remove permutations to be
 able to add new ones?  E.g., why can't we start using soft 
 permutationshttps://code.google.com/p/google-web-toolkit/wiki/SoftPermutationsby
  default to limit permutation growth from old or similar permutations?

 I'm also curious what reasons you had in mind for needing a Blink-specific
 permutation in GWT.  Chromium and Firefox have policies now to avoid
 introducing vendor prefixes for new functionality, so a Blink permutation
 seems limited in use to working around bugs or performance tuning.

 Not to mention GWT's release and update cycles are much slower than
 Chromium and Firefox's, so it's likely our tweaks/workarounds are
 irrelevant before GWT users can start utilizing them. ;)

 --
 You received this message because you are subscribed to the Google Groups
 GWT Steering group.
 To unsubscribe from this group and stop receiving emails from it, send an
 email to gwt-steering+unsubscr...@googlegroups.com.
 For more options, visit https://groups.google.com/groups/opt_out.




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT 
Contributors group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Re: Removing support for old browsers

2013-05-30 Thread Daniel Kurka
One example might be the startup performance improvement stuff I looked
into. Which is faster on any browser except chrome, so we might want to do
something in a v8 specific permutation instead of doing that to safari as
well.


On Thu, May 30, 2013 at 10:37 PM, Ray Cromwell cromwell...@google.comwrote:

 One reason to have a permutation for blink is if we start doing VM
 specific code gen (V8 vs Nitro vs *Monkey).  Another is blink performance
 around layout might require substantially different workarounds on Mobile
 Safari.



 On Mon, May 27, 2013 at 11:42 AM, Matthew Dempsky mdemp...@google.comwrote:

 [-gwt-steering, +gwt-contrib]

 On Mon, May 27, 2013 at 9:32 AM, Daniel Kurka danku...@google.comwrote:

 We also need the permutations for blink.


 Are there technical reasons for why we need to remove permutations to be
 able to add new ones?  E.g., why can't we start using soft 
 permutationshttps://code.google.com/p/google-web-toolkit/wiki/SoftPermutationsby
  default to limit permutation growth from old or similar permutations?

 I'm also curious what reasons you had in mind for needing a
 Blink-specific permutation in GWT.  Chromium and Firefox have policies now
 to avoid introducing vendor prefixes for new functionality, so a Blink
 permutation seems limited in use to working around bugs or performance
 tuning.

 Not to mention GWT's release and update cycles are much slower than
 Chromium and Firefox's, so it's likely our tweaks/workarounds are
 irrelevant before GWT users can start utilizing them. ;)

 --
 You received this message because you are subscribed to the Google Groups
 GWT Steering group.
 To unsubscribe from this group and stop receiving emails from it, send an
 email to gwt-steering+unsubscr...@googlegroups.com.
 For more options, visit https://groups.google.com/groups/opt_out.




  --
 You received this message because you are subscribed to the Google Groups
 GWT Steering group.
 To unsubscribe from this group and stop receiving emails from it, send an
 email to gwt-steering+unsubscr...@googlegroups.com.
 For more options, visit https://groups.google.com/groups/opt_out.






-- 
Google Germany GmbH
*Dienerstr. 12*
*80331 München*

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Katherine Stephens

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT 
Contributors group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Inliner produces incorrect code if a parameter is a new arra...

2013-05-30 Thread Roberto Lublinerman

Roberto Lublinerman has submitted this change and it was merged.

Change subject: Inliner produces incorrect code if a parameter is a new  
array expression.

..


Inliner produces incorrect code if a parameter is a new array expression.

The inliner incorrectly inlines a call when a parameter is a new array
expression resulting in the new array contruct beign duplicated if more
that one reference to such parameter is present.

Fixes issue 6638.

Change-Id: Ic083ac15d7d84ab4728c441fd590977cd684a87a
---
M dev/core/src/com/google/gwt/dev/jjs/impl/ExpressionAnalyzer.java
M dev/core/test/com/google/gwt/dev/jjs/impl/ExpressionAnalyzerTest.java
M user/test/com/google/gwt/dev/jjs/test/CompilerMiscRegressionTest.java
3 files changed, 27 insertions(+), 0 deletions(-)

Approvals:
  Ray Cromwell: Looks good to me, approved
  Leeroy Jenkins: Verified



diff --git  
a/dev/core/src/com/google/gwt/dev/jjs/impl/ExpressionAnalyzer.java  
b/dev/core/src/com/google/gwt/dev/jjs/impl/ExpressionAnalyzer.java

index 2b17399..3be97ad 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/ExpressionAnalyzer.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ExpressionAnalyzer.java
@@ -177,6 +177,7 @@

   @Override
   public void endVisit(JNewArray x, Context ctx) {
+createsObject = true;
 /*
  * If no array bounds, the new array is being automatically  
initialized. If
  * there are side-effects, they'll show up when we visit the  
initializers.
diff --git  
a/dev/core/test/com/google/gwt/dev/jjs/impl/ExpressionAnalyzerTest.java  
b/dev/core/test/com/google/gwt/dev/jjs/impl/ExpressionAnalyzerTest.java

index 7cf1909..fb8bae5 100644
--- a/dev/core/test/com/google/gwt/dev/jjs/impl/ExpressionAnalyzerTest.java
+++ b/dev/core/test/com/google/gwt/dev/jjs/impl/ExpressionAnalyzerTest.java
@@ -168,6 +168,10 @@
 analyzeExpression(boolean, FOO =  
false).accessesFieldNonFinal().hasAssignmentToField().check();

   }

+  public void testNewArray() throws Exception {
+analyzeExpression(float[], new float[3]).createsObject().check();
+  }
+
   private Result analyzeExpression(String type, String expression)
   throws UnableToCompleteException {
 JProgram program = compileSnippet(type, return  + expression + ;);
diff --git  
a/user/test/com/google/gwt/dev/jjs/test/CompilerMiscRegressionTest.java  
b/user/test/com/google/gwt/dev/jjs/test/CompilerMiscRegressionTest.java

index 841eec3..0ce2cd0 100644
--- a/user/test/com/google/gwt/dev/jjs/test/CompilerMiscRegressionTest.java
+++ b/user/test/com/google/gwt/dev/jjs/test/CompilerMiscRegressionTest.java
@@ -52,4 +52,26 @@
 assertEquals(12.0, addAndConvert(10, 2));
 assertEquals(-10.0, minusAndDecrement(11));
   }
+  private static float[] copy(float[] src, float[] dest) {
+System.arraycopy(src, 0, dest, 0, Math.min(src.length, dest.length));
+return dest;
+  }
+
+  /**
+   * Test for issue 6638.
+   */
+  public void testNewArrayInlining() {
+float[] src = new float[]{1,1,1};
+float[] dest = copy(src, new float[3]);
+
+assertEqualContents(src, dest);
+  }
+
+  private static void assertEqualContents(float[] expected, float[]  
actual) {

+
+assertEquals(Array length mismatch, expected.length, actual.length);
+for (int i = 0; i  expected.length; i++) {
+  assertEquals(Array mismatch at element  + i , expected[i],  
actual[i]);

+}
+  }
 }

--
To view, visit https://gwt-review.googlesource.com/3040
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ic083ac15d7d84ab4728c441fd590977cd684a87a
Gerrit-PatchSet: 2
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Roberto Lublinerman rlu...@google.com
Gerrit-Reviewer: Leeroy Jenkins jenk...@gwtproject.org
Gerrit-Reviewer: Matthew Dempsky mdemp...@google.com
Gerrit-Reviewer: Ray Cromwell cromwell...@google.com
Gerrit-Reviewer: Roberto Lublinerman rlu...@google.com

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Inliner produces incorrect code if a parameter is a new arra...

2013-05-30 Thread Roberto Lublinerman

Roberto Lublinerman has posted comments on this change.

Change subject: Inliner produces incorrect code if a parameter is a new  
array expression.

..


Patch Set 2:

Well at list now JNewArray is treated exactly as JNewInstance. We could  
definitely improve the inliner to be less conservative.


--
To view, visit https://gwt-review.googlesource.com/3040
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic083ac15d7d84ab4728c441fd590977cd684a87a
Gerrit-PatchSet: 2
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Roberto Lublinerman rlu...@google.com
Gerrit-Reviewer: Leeroy Jenkins jenk...@gwtproject.org
Gerrit-Reviewer: Matthew Dempsky mdemp...@google.com
Gerrit-Reviewer: Ray Cromwell cromwell...@google.com
Gerrit-Reviewer: Roberto Lublinerman rlu...@google.com
Gerrit-HasComments: No

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Fix module unloading with multiple modules on a page

2013-05-30 Thread Ray Cromwell

Ray Cromwell has posted comments on this change.

Change subject: Fix module unloading with multiple modules on a page
..


Patch Set 1: Code-Review+2

--
To view, visit https://gwt-review.googlesource.com/2941
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I25d9471c1e14756196a11223eadb765ed303a3b4
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Matthew Dempsky mdemp...@google.com
Gerrit-Reviewer: Leeroy Jenkins jenk...@gwtproject.org
Gerrit-Reviewer: Matthew Dempsky mdemp...@google.com
Gerrit-Reviewer: Ray Cromwell cromwell...@google.com
Gerrit-Reviewer: Thomas Broyer t.bro...@gmail.com
Gerrit-HasComments: No

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Fixes TypeError in keys() for JsMapFromStringTo in dev mode.

2013-05-30 Thread Ray Cromwell

Ray Cromwell has posted comments on this change.

Change subject: Fixes TypeError in keys() for JsMapFromStringTo in dev mode.
..


Patch Set 1:

(1 comment)


File elemental/src/elemental/js/util/JsMapFromStringTo.java
Line 42:   if (Object.prototype.hasOwnProperty.call(object, item)) {
Yes, technically we could do this. I think I need to re-evaluate the  
Object.create(null) trick. The original idea was to have a true map that  
doesn't need a key prefix to avoid collisions and have a pure get/put that  
works.


The idea in Elemental is that these primitive interfaces can be cross-cast.  
e.g. element.getAttributes() could be cast to JsMapFromStringToString, and  
you could just operate on the values like any other map. Likewise, a  
JsonObject could be cast to JsMapFromStringToString (unsafely). So a  
{x: foo, y:bar} object literal could be both a JsonObject and a  
JsMapFromStringToString.


Thus, in the latter case, we still need the hasOwnProperty checks, because  
the object may be created by cross-cast elsewhere.



--
To view, visit https://gwt-review.googlesource.com/1680
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5d0c2943c94c1119ae423dfbda6c932045e0b9
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: James Nelson ja...@wetheinter.net
Gerrit-Reviewer: James Nelson ja...@wetheinter.net
Gerrit-Reviewer: Ray Cromwell cromwell...@google.com
Gerrit-Reviewer: Thomas Broyer t.bro...@gmail.com
Gerrit-HasComments: Yes

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Add hasClassName method in com.google.gwt.dom.client.Element

2013-05-30 Thread Stephen Haberman

Stephen Haberman has posted comments on this change.

Change subject: Add hasClassName method in com.google.gwt.dom.client.Element
..


Patch Set 4:

(1 comment)


File user/src/com/google/gwt/dom/client/Element.java
Line 590:   private static String trimClassName(String className) {
I believe we still put static methods at the top of the class.


--
To view, visit https://gwt-review.googlesource.com/3070
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia09567b8c58cac02f8126c33ef169b26def3d19c
Gerrit-PatchSet: 4
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Andrey Korzhevskiy a.korzhevs...@gmail.com
Gerrit-Reviewer: Colin Alworth niloc...@gmail.com
Gerrit-Reviewer: Daniel Kurka danku...@google.com
Gerrit-Reviewer: Goktug Gokdogan gok...@google.com
Gerrit-Reviewer: Stephen Haberman stephen.haber...@gmail.com
Gerrit-Reviewer: Thomas Broyer t.bro...@gmail.com
Gerrit-HasComments: Yes

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Workaround for resolving mouse button on mouse move in Firefox

2013-05-30 Thread Goktug Gokdogan

Goktug Gokdogan has posted comments on this change.

Change subject: Workaround for resolving mouse button on mouse move in  
Firefox

..


Patch Set 3:

Himm, I'm not very knowledgeable on browser quirks; will this work as  
expected in all Firefox versions?


--
To view, visit https://gwt-review.googlesource.com/2330
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib73997af56ce0e7da5b41814a7ac2b208ab022aa
Gerrit-PatchSet: 3
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: John Ahlroos j...@vaadin.com
Gerrit-Reviewer: Goktug Gokdogan gok...@google.com
Gerrit-Reviewer: John Ahlroos j...@vaadin.com
Gerrit-Reviewer: Leeroy Jenkins jenk...@gwtproject.org
Gerrit-Reviewer: Matthew Dempsky mdemp...@google.com
Gerrit-Reviewer: Matthew Dempsky mdemp...@gwtproject.org
Gerrit-HasComments: No

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Add hasClassName method in com.google.gwt.dom.client.Element

2013-05-30 Thread Goktug Gokdogan

Goktug Gokdogan has posted comments on this change.

Change subject: Add hasClassName method in com.google.gwt.dom.client.Element
..


Patch Set 4: Code-Review+1

(1 comment)


File user/src/com/google/gwt/dom/client/Element.java
Line 590:   private static String trimClassName(String className) {
That rule is already broken in this class. I'll start a thread in gwt  
contrib for an update to the style guide.



--
To view, visit https://gwt-review.googlesource.com/3070
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia09567b8c58cac02f8126c33ef169b26def3d19c
Gerrit-PatchSet: 4
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Andrey Korzhevskiy a.korzhevs...@gmail.com
Gerrit-Reviewer: Colin Alworth niloc...@gmail.com
Gerrit-Reviewer: Daniel Kurka danku...@google.com
Gerrit-Reviewer: Goktug Gokdogan gok...@google.com
Gerrit-Reviewer: Stephen Haberman stephen.haber...@gmail.com
Gerrit-Reviewer: Thomas Broyer t.bro...@gmail.com
Gerrit-HasComments: Yes

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Update how columns are hidden in CellTable.

2013-05-30 Thread Goktug Gokdogan

Goktug Gokdogan has uploaded a new change for review.

  https://gwt-review.googlesource.com/3130


Change subject: Update how columns are hidden in CellTable.
..

Update how columns are hidden in CellTable.

Hide columns that shouldn't be visible by setting display to none.
This is to fix a rendering issue in safari and chrome, where the column
is hidden, but doesn't allow the other columns to expand to fill the
remaining space.

Original patch by: Aliseya Wright

Change-Id: I49fd531a02ca0a1a55eabba5548670ad86273a43
---
M user/src/com/google/gwt/user/cellview/client/CellTable.java
A user/src/com/google/gwt/user/client/ui/FocusComposite.java
A user/src/com/google/gwt/user/client/ui/PanelComposite.java
3 files changed, 305 insertions(+), 5 deletions(-)



diff --git a/user/src/com/google/gwt/user/cellview/client/CellTable.java  
b/user/src/com/google/gwt/user/cellview/client/CellTable.java

index 12b3db2..b2cd03e 100644
--- a/user/src/com/google/gwt/user/cellview/client/CellTable.java
+++ b/user/src/com/google/gwt/user/cellview/client/CellTable.java
@@ -824,11 +824,10 @@
 // enabled), and refreshColumnWidth/clearColumnWidth. The latter two  
are no op if setColumnWidth

 // is not invoked first.
 if (colGroupEnabled) {
-  if (width == null) {
-ensureTableColElement(column).getStyle().clearWidth();
-  } else {
-ensureTableColElement(column).getStyle().setProperty(width,  
width);

-  }
+  TableColElement columnElement = ensureTableColElement(column);
+  columnElement.getStyle().setProperty(width, width == null ?  :  
width);

+  boolean isColumnVisible = !0.equals(width)  !0px.equals(width);
+  columnElement.getStyle().setProperty(display,  
isColumnVisible ?  : none);

 }
   }

diff --git a/user/src/com/google/gwt/user/client/ui/FocusComposite.java  
b/user/src/com/google/gwt/user/client/ui/FocusComposite.java

new file mode 100644
index 000..0173bb0
--- /dev/null
+++ b/user/src/com/google/gwt/user/client/ui/FocusComposite.java
@@ -0,0 +1,229 @@
+/*
+ * Copyright 2013 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the License); you may  
not
+ * use this file except in compliance with the License. You may obtain a  
copy of

+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,  
WITHOUT

+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations  
under

+ * the License.
+ */
+package com.google.gwt.user.client.ui;
+
+import com.google.gwt.event.dom.client.BlurHandler;
+import com.google.gwt.event.dom.client.ClickHandler;
+import com.google.gwt.event.dom.client.DoubleClickHandler;
+import com.google.gwt.event.dom.client.DragEndHandler;
+import com.google.gwt.event.dom.client.DragEnterHandler;
+import com.google.gwt.event.dom.client.DragHandler;
+import com.google.gwt.event.dom.client.DragLeaveHandler;
+import com.google.gwt.event.dom.client.DragOverHandler;
+import com.google.gwt.event.dom.client.DragStartHandler;
+import com.google.gwt.event.dom.client.DropHandler;
+import com.google.gwt.event.dom.client.FocusHandler;
+import com.google.gwt.event.dom.client.GestureChangeHandler;
+import com.google.gwt.event.dom.client.GestureEndHandler;
+import com.google.gwt.event.dom.client.GestureStartHandler;
+import com.google.gwt.event.dom.client.HasAllDragAndDropHandlers;
+import com.google.gwt.event.dom.client.HasAllFocusHandlers;
+import com.google.gwt.event.dom.client.HasAllGestureHandlers;
+import com.google.gwt.event.dom.client.HasAllKeyHandlers;
+import com.google.gwt.event.dom.client.HasAllMouseHandlers;
+import com.google.gwt.event.dom.client.HasAllTouchHandlers;
+import com.google.gwt.event.dom.client.HasClickHandlers;
+import com.google.gwt.event.dom.client.HasDoubleClickHandlers;
+import com.google.gwt.event.dom.client.KeyDownHandler;
+import com.google.gwt.event.dom.client.KeyPressHandler;
+import com.google.gwt.event.dom.client.KeyUpHandler;
+import com.google.gwt.event.dom.client.MouseDownHandler;
+import com.google.gwt.event.dom.client.MouseMoveHandler;
+import com.google.gwt.event.dom.client.MouseOutHandler;
+import com.google.gwt.event.dom.client.MouseOverHandler;
+import com.google.gwt.event.dom.client.MouseUpHandler;
+import com.google.gwt.event.dom.client.MouseWheelHandler;
+import com.google.gwt.event.dom.client.TouchCancelHandler;
+import com.google.gwt.event.dom.client.TouchEndHandler;
+import com.google.gwt.event.dom.client.TouchMoveHandler;
+import com.google.gwt.event.dom.client.TouchStartHandler;
+import com.google.gwt.event.shared.HandlerRegistration;
+
+/**
+ * A {@link Composite} that wraps a {@link Focusable} widget.
+ *
+ * @param T type of the widget wrapped
+ */
+public class 

[gwt-contrib] Change in gwt[master]: Update how columns are hidden in CellTable.

2013-05-30 Thread Jens Nehlmeier

Jens Nehlmeier has posted comments on this change.

Change subject: Update how columns are hidden in CellTable.
..


Patch Set 1:

(2 comments)

You want to remove your both Composites from this CL :-)


File user/src/com/google/gwt/user/cellview/client/CellTable.java
Line 828:   columnElement.getStyle().setProperty(width, width ==  
null ?  : width);

columnElement.setWidth(...) doesn't work here?


Line 830:   columnElement.getStyle().setProperty(display,  
isColumnVisible ?  : none);
You can use setVisible(columnElement, isColumnVisible) which does the same  
and also sets the aria-hidden attribute.



--
To view, visit https://gwt-review.googlesource.com/3130
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I49fd531a02ca0a1a55eabba5548670ad86273a43
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan gok...@google.com
Gerrit-Reviewer: Jens Nehlmeier jens.nehlme...@gmail.com
Gerrit-Reviewer: Leeroy Jenkins jenk...@gwtproject.org
Gerrit-HasComments: Yes

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Update how columns are hidden in CellTable.

2013-05-30 Thread Goktug Gokdogan

Goktug Gokdogan has posted comments on this change.

Change subject: Update how columns are hidden in CellTable.
..


Patch Set 1:

No, this exactly where I want them to be :-P

I guess I have some script trouble. Will send it again...

--
To view, visit https://gwt-review.googlesource.com/3130
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I49fd531a02ca0a1a55eabba5548670ad86273a43
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan gok...@google.com
Gerrit-Reviewer: Goktug Gokdogan gok...@google.com
Gerrit-Reviewer: Jens Nehlmeier jens.nehlme...@gmail.com
Gerrit-Reviewer: Leeroy Jenkins jenk...@gwtproject.org
Gerrit-HasComments: No

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Update how columns are hidden in CellTable.

2013-05-30 Thread Goktug Gokdogan

Goktug Gokdogan has posted comments on this change.

Change subject: Update how columns are hidden in CellTable.
..


Patch Set 1:

(2 comments)


File user/src/com/google/gwt/user/cellview/client/CellTable.java
Line 828:   columnElement.getStyle().setProperty(width, width ==  
null ?  : width);

not supported in HTML5


Line 830:   columnElement.getStyle().setProperty(display,  
isColumnVisible ?  : none);

Sounds good. Will do.


--
To view, visit https://gwt-review.googlesource.com/3130
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I49fd531a02ca0a1a55eabba5548670ad86273a43
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan gok...@google.com
Gerrit-Reviewer: Goktug Gokdogan gok...@google.com
Gerrit-Reviewer: Jens Nehlmeier jens.nehlme...@gmail.com
Gerrit-Reviewer: Leeroy Jenkins jenk...@gwtproject.org
Gerrit-HasComments: Yes

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: ActivityManager fails to clear Eventbus

2013-05-30 Thread Jens Nehlmeier

Jens Nehlmeier has posted comments on this change.

Change subject: ActivityManager fails to clear Eventbus
..


Patch Set 1:

Maybe adding an explicit dispose() method would be suitable. Calling  
dispose() will set the display to null and tear down any active activity.


With such a dispose() method we could even disallow calling setDisplay()  
with null. That would mean you could only swap displays which would trigger  
an activity restart as Goktug described it. This would tighten up the API a  
bit but I can only hardly see a use case where you want to call  
setDisplay() with null other than for cleanup.


--
To view, visit https://gwt-review.googlesource.com/2974
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3ffb6911b0723969001bd43c73059b4dfad104cf
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Daniel Kurka danku...@google.com
Gerrit-Reviewer: Goktug Gokdogan gok...@google.com
Gerrit-Reviewer: Jens Nehlmeier jens.nehlme...@gmail.com
Gerrit-Reviewer: Leeroy Jenkins jenk...@gwtproject.org
Gerrit-Reviewer: Thomas Broyer t.bro...@gmail.com
Gerrit-HasComments: No

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Update how columns are hidden in CellTable.

2013-05-30 Thread Jens Nehlmeier

Jens Nehlmeier has posted comments on this change.

Change subject: Update how columns are hidden in CellTable.
..


Patch Set 1:

(1 comment)


File user/src/com/google/gwt/user/cellview/client/CellTable.java
Line 828:   columnElement.getStyle().setProperty(width, width ==  
null ?  : width);
Uh, then TableColElement.setWidth() should be reimplemented using styles  
asap and finally called here?



--
To view, visit https://gwt-review.googlesource.com/3130
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I49fd531a02ca0a1a55eabba5548670ad86273a43
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan gok...@google.com
Gerrit-Reviewer: Goktug Gokdogan gok...@google.com
Gerrit-Reviewer: Jens Nehlmeier jens.nehlme...@gmail.com
Gerrit-Reviewer: Leeroy Jenkins jenk...@gwtproject.org
Gerrit-HasComments: Yes

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Ensures integer pixel values and adds getters for subpixel v...

2013-05-30 Thread Brian Slesinsky

Brian Slesinsky has posted comments on this change.

Change subject: Ensures integer pixel values and adds getters for subpixel  
values

..


Patch Set 1:

There are 50+ subclasses. How many of them will need subpixel methods too?  
It looks like ImageElement might.


Putting the methods directly on Element seems a bit more subclass-friendly;  
otherwise you need a different getSubpixel() method for each subclass.


Since it's a JSO, I suppose we could also start over with a new class  
hierarchy and freely cast between the old and new classes. But that's a  
huge change.


--
To view, visit https://gwt-review.googlesource.com/3090
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I27e98f0ad3c1c236b2d85d24f197719fa4cd352a
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: John Ahlroos j...@vaadin.com
Gerrit-Reviewer: Artur Signell ar...@vaadin.com
Gerrit-Reviewer: Brian Slesinsky skybr...@google.com
Gerrit-Reviewer: Goktug Gokdogan gok...@google.com
Gerrit-Reviewer: Leeroy Jenkins jenk...@gwtproject.org
Gerrit-Reviewer: Thomas Broyer t.bro...@gmail.com
Gerrit-HasComments: No

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Update how columns are hidden in CellTable.

2013-05-30 Thread Goktug Gokdogan

Goktug Gokdogan has posted comments on this change.

Change subject: Update how columns are hidden in CellTable.
..


Patch Set 1:

(1 comment)


File user/src/com/google/gwt/user/cellview/client/CellTable.java
Line 828:   columnElement.getStyle().setProperty(width, width ==  
null ?  : width);

AFAIK, there are other similar attributes deprecated by HTML5.

Element APIs are very close to DOM. My current intention is to eventually  
deprecate and remove them instead of emulating.



--
To view, visit https://gwt-review.googlesource.com/3130
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I49fd531a02ca0a1a55eabba5548670ad86273a43
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan gok...@google.com
Gerrit-Reviewer: Goktug Gokdogan gok...@google.com
Gerrit-Reviewer: Jens Nehlmeier jens.nehlme...@gmail.com
Gerrit-Reviewer: Leeroy Jenkins jenk...@gwtproject.org
Gerrit-HasComments: Yes

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Ensures integer pixel values and adds getters for subpixel v...

2013-05-30 Thread Goktug Gokdogan

Goktug Gokdogan has posted comments on this change.

Change subject: Ensures integer pixel values and adds getters for subpixel  
values

..


Patch Set 1:

Putting the methods directly on Element seems a bit more  
subclass-friendly; otherwise you need a different getSubpixel() method  
for each subclass.


Why do we need a different getSubpixel method each subclass?

--
To view, visit https://gwt-review.googlesource.com/3090
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I27e98f0ad3c1c236b2d85d24f197719fa4cd352a
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: John Ahlroos j...@vaadin.com
Gerrit-Reviewer: Artur Signell ar...@vaadin.com
Gerrit-Reviewer: Brian Slesinsky skybr...@google.com
Gerrit-Reviewer: Goktug Gokdogan gok...@google.com
Gerrit-Reviewer: Leeroy Jenkins jenk...@gwtproject.org
Gerrit-Reviewer: Thomas Broyer t.bro...@gmail.com
Gerrit-HasComments: No

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Update how columns are hidden in CellTable.

2013-05-30 Thread Goktug Gokdogan

Hello Leeroy Jenkins,

I'd like you to reexamine a change.  Please visit

https://gwt-review.googlesource.com/3130

to look at the new patch set (#2).

Change subject: Update how columns are hidden in CellTable.
..

Update how columns are hidden in CellTable.

Hide columns that shouldn't be visible by setting display to none.
This is to fix a rendering issue in safari and chrome, where the column
is hidden, but doesn't allow the other columns to expand to fill the
remaining space.

Original patch by: Aliseya Wright

Change-Id: I49fd531a02ca0a1a55eabba5548670ad86273a43
Review-Link: https://gwt-review.googlesource.com/#/c/3130/
---
M user/src/com/google/gwt/user/cellview/client/CellTable.java
1 file changed, 5 insertions(+), 5 deletions(-)


--
To view, visit https://gwt-review.googlesource.com/3130
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I49fd531a02ca0a1a55eabba5548670ad86273a43
Gerrit-PatchSet: 2
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan gok...@google.com
Gerrit-Reviewer: Goktug Gokdogan gok...@google.com
Gerrit-Reviewer: Jens Nehlmeier jens.nehlme...@gmail.com
Gerrit-Reviewer: Leeroy Jenkins jenk...@gwtproject.org

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Change in gwt[master]: Removes delegation of event from TextBox event handler insid...

2013-05-30 Thread Goktug Gokdogan

Goktug Gokdogan has uploaded a new change for review.

  https://gwt-review.googlesource.com/3141


Change subject: Removes delegation of event from TextBox event handler  
inside SuggestBox.

..

Removes delegation of event from TextBox event handler inside SuggestBox.

The events are already fired in SuggestBox as it is a composite, additional
delegation was causing events to be fired twice.

Bug: Issue 3533

Original author: Patrick Tucker

Change-Id: I70752f24667a83bee208f31f37ec63d79ec8b1b8
---
M user/src/com/google/gwt/user/client/ui/SuggestBox.java
1 file changed, 3 insertions(+), 10 deletions(-)



diff --git a/user/src/com/google/gwt/user/client/ui/SuggestBox.java  
b/user/src/com/google/gwt/user/client/ui/SuggestBox.java

index 818e9b2..7d87fb2 100644
--- a/user/src/com/google/gwt/user/client/ui/SuggestBox.java
+++ b/user/src/com/google/gwt/user/client/ui/SuggestBox.java
@@ -21,7 +21,6 @@
 import com.google.gwt.editor.client.IsEditor;
 import com.google.gwt.editor.client.LeafValueEditor;
 import com.google.gwt.editor.client.adapters.TakesValueEditor;
-import com.google.gwt.event.dom.client.HandlesAllKeyEvents;
 import com.google.gwt.event.dom.client.HasAllKeyHandlers;
 import com.google.gwt.event.dom.client.KeyCodes;
 import com.google.gwt.event.dom.client.KeyDownEvent;
@@ -1119,8 +1118,7 @@
   }

   private void addEventsToTextBox() {
-class TextBoxEvents extends HandlesAllKeyEvents implements
-ValueChangeHandlerString {
+class TextBoxEvents implements KeyDownHandler, KeyUpHandler,  
ValueChangeHandlerString {


   public void onKeyDown(KeyDownEvent event) {
 switch (event.getNativeKeyCode()) {
@@ -1140,17 +1138,11 @@
 }
 break;
 }
-delegateEvent(SuggestBox.this, event);
-  }
-
-  public void onKeyPress(KeyPressEvent event) {
-delegateEvent(SuggestBox.this, event);
   }

   public void onKeyUp(KeyUpEvent event) {
 // After every user key input, refresh the popup's suggestions.
 refreshSuggestions();
-delegateEvent(SuggestBox.this, event);
   }

   public void onValueChange(ValueChangeEventString event) {
@@ -1159,7 +1151,8 @@
 }

 TextBoxEvents events = new TextBoxEvents();
-events.addKeyHandlersTo(box);
+box.addKeyDownHandler(events);
+box.addKeyUpHandler(events);
 box.addValueChangeHandler(events);
   }


--
To view, visit https://gwt-review.googlesource.com/3141
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I70752f24667a83bee208f31f37ec63d79ec8b1b8
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan gok...@google.com

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] Re: SuggestBox causes native events to fire twice (3533) (issue1785803)

2013-05-30 Thread goktug

https://gwt-review.googlesource.com/#/c/3141/

On 2013/05/30 12:45:08, Patrick Tucker wrote:

If you can finish it up that would be great.



I haven't really had time to figure out the new system.  With the old

system I

could generate patch files from any of the many computers that I work

on, email

it to myself and submit the patch from anywhere.  I can't really do

that

anymore...



On 2013/05/29 20:41:32, goktug wrote:
 On 2012/09/05 15:44:32, Patrick Tucker wrote:
  On 2012/07/19 20:48:12, tbroyer wrote:
 
  Can this patch be committed?

 Can you move this patch to gerrit or would you prefer me to do that

for you?


http://gwt-code-reviews.appspot.com/1785803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




[gwt-contrib] GWT Coding Style Update

2013-05-30 Thread Goktug Gokdogan
Hi folks.

We currently have a coding style guide for GWT-SDK here:
  https://developers.google.com/web-toolkit/makinggwtbetter#codestyle
The guide has some enforcements like static/non-static + alphabetical
ordering. Alphabetical ordering is as good as no ordering in many cases and
these rules are already broken in many places.

I propose following Guava and many other java libs and just enforce logical
ordering during code reviews (which is also internal recommendation at
Google). One good approach for logical order is to group related public
APIs and then slot in other methods and nested-classes to keep them close
to the related context. This usually helps to follow the code with less
jumps around the class.

Anyway, I know this can quickly turn into a bikeshed discussion so please
try to resist the urge to oppose unless you have a big concern =)

Cheers!

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT 
Contributors group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




Re: [gwt-contrib] GWT Coding Style Update

2013-05-30 Thread John A. Tamplin
On Thu, May 30, 2013 at 9:22 PM, Goktug Gokdogan gok...@google.com wrote:

 We currently have a coding style guide for GWT-SDK here:
   https://developers.google.com/web-toolkit/makinggwtbetter#codestyle
 The guide has some enforcements like static/non-static + alphabetical
 ordering. Alphabetical ordering is as good as no ordering in many cases and
 these rules are already broken in many places.

 I propose following Guava and many other java libs and just enforce
 logical ordering during code reviews (which is also internal recommendation
 at Google). One good approach for logical order is to group related public
 APIs and then slot in other methods and nested-classes to keep them close
 to the related context. This usually helps to follow the code with less
 jumps around the class.

 Anyway, I know this can quickly turn into a bikeshed discussion so please
 try to resist the urge to oppose unless you have a big concern =)


I have worked in the GWT codebase for years, and in others that didn't have
as strict style/order guidelines, and I have to say I *greatly* prefer
working in the current code. Even with IDEs letting you find members more
easily, I still really miss having ordered method names when I work in code
that doesn't have it.

-- 
John A. Tamplin

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT 
Contributors group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




Re: [gwt-contrib] GWT Coding Style Update

2013-05-30 Thread John Stalcup
I'm a big fan of enforcement of static/non-static + alphabetical ordering,
as it stays consistent over time and can be automatically performed by
IDEs. I made it the rule in my last team.

I wish we were actually enforcing the GWT-SDK style rules!


On Thu, May 30, 2013 at 6:22 PM, Goktug Gokdogan gok...@google.com wrote:

 Hi folks.

 We currently have a coding style guide for GWT-SDK here:
   https://developers.google.com/web-toolkit/makinggwtbetter#codestyle
 The guide has some enforcements like static/non-static + alphabetical
 ordering. Alphabetical ordering is as good as no ordering in many cases and
 these rules are already broken in many places.

 I propose following Guava and many other java libs and just enforce
 logical ordering during code reviews (which is also internal recommendation
 at Google). One good approach for logical order is to group related public
 APIs and then slot in other methods and nested-classes to keep them close
 to the related context. This usually helps to follow the code with less
 jumps around the class.

 Anyway, I know this can quickly turn into a bikeshed discussion so please
 try to resist the urge to oppose unless you have a big concern =)

 Cheers!

 --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors
 ---
 You received this message because you are subscribed to the Google Groups
 GWT Contributors group.
 To unsubscribe from this group and stop receiving emails from it, send an
 email to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
 For more options, visit https://groups.google.com/groups/opt_out.




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT 
Contributors group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




Re: [gwt-contrib] GWT Coding Style Update

2013-05-30 Thread Goktug Gokdogan
At the end I guess this comes to personal preferences.
If you personally prefer the static/non-static + alphabetical ordering, IDE
can show it to you with a simple keystroke (Ctrl+O in Eclipse) but it can't
do the reverse.


On Thu, May 30, 2013 at 7:13 PM, John Stalcup stal...@google.com wrote:

 I'm a big fan of enforcement of static/non-static + alphabetical
 ordering, as it stays consistent over time and can be automatically
 performed by IDEs. I made it the rule in my last team.

 I wish we were actually enforcing the GWT-SDK style rules!


 On Thu, May 30, 2013 at 6:22 PM, Goktug Gokdogan gok...@google.comwrote:

 Hi folks.

 We currently have a coding style guide for GWT-SDK here:
   https://developers.google.com/web-toolkit/makinggwtbetter#codestyle
 The guide has some enforcements like static/non-static + alphabetical
 ordering. Alphabetical ordering is as good as no ordering in many cases and
 these rules are already broken in many places.

 I propose following Guava and many other java libs and just enforce
 logical ordering during code reviews (which is also internal recommendation
 at Google). One good approach for logical order is to group related public
 APIs and then slot in other methods and nested-classes to keep them close
 to the related context. This usually helps to follow the code with less
 jumps around the class.

 Anyway, I know this can quickly turn into a bikeshed discussion so please
 try to resist the urge to oppose unless you have a big concern =)

 Cheers!

 --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors
 ---
 You received this message because you are subscribed to the Google Groups
 GWT Contributors group.
 To unsubscribe from this group and stop receiving emails from it, send an
 email to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
 For more options, visit https://groups.google.com/groups/opt_out.




  --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors
 ---
 You received this message because you are subscribed to the Google Groups
 GWT Contributors group.
 To unsubscribe from this group and stop receiving emails from it, send an
 email to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
 For more options, visit https://groups.google.com/groups/opt_out.




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT 
Contributors group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




Re: [gwt-contrib] GWT Coding Style Update

2013-05-30 Thread John A. Tamplin
On Thu, May 30, 2013 at 10:22 PM, Goktug Gokdogan gok...@google.com wrote:

 At the end I guess this comes to personal preferences.
 If you personally prefer the static/non-static + alphabetical ordering,
 IDE can show it to you with a simple keystroke (Ctrl+O in Eclipse) but it
 can't do the reverse.


So what is the rationale behind this proposal?  GWT has operated the way it
is for many years.  What problems has it caused?

Are we no longer running ant checkstyle in the submit queue?

-- 
John A. Tamplin

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT 
Contributors group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




Re: [gwt-contrib] GWT Coding Style Update

2013-05-30 Thread Goktug Gokdogan
It is currently inconsistent, it is not enforced, most of GWT contributions
are coming from Googlers who are not aware that GWT has different rules
than all other Google3 code. Moving code from Google3 to GWT requires
re-sorting. If you change the name of a function, you need to move the code
to a new random location and on the review you can't see what really
changed in the function or even if it has changed. I don't care much about
internal stuff much but seeing an API in alphabetical order is as good as
any other random order for the end users of the SDK.
These are the similar reasons why none of other high profile libs (Guava,
Guice, Dagger and JDK itself) enforce such rules.



On Thu, May 30, 2013 at 7:42 PM, John A. Tamplin j...@jaet.org wrote:

 On Thu, May 30, 2013 at 10:22 PM, Goktug Gokdogan gok...@google.comwrote:

 At the end I guess this comes to personal preferences.
 If you personally prefer the static/non-static + alphabetical ordering,
 IDE can show it to you with a simple keystroke (Ctrl+O in Eclipse) but
 it can't do the reverse.


 So what is the rationale behind this proposal?  GWT has operated the way
 it is for many years.  What problems has it caused?

 Are we no longer running ant checkstyle in the submit queue?

 --
 John A. Tamplin

 --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors
 ---
 You received this message because you are subscribed to the Google Groups
 GWT Contributors group.
 To unsubscribe from this group and stop receiving emails from it, send an
 email to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
 For more options, visit https://groups.google.com/groups/opt_out.




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT 
Contributors group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




Re: [gwt-contrib] GWT Coding Style Update

2013-05-30 Thread John A. Tamplin
On Thu, May 30, 2013 at 11:13 PM, Goktug Gokdogan gok...@google.com wrote:

 It is currently inconsistent, it is not enforced,


Why was it stopped being enforced?  It used to run :ant_tests (IIRC) that
ran checkstyle, and TAP definitely caught style violations.  We never had
any trouble accepting internal contributions when I was there, as it would
get caught in the submit queue.


 most of GWT contributions are coming from Googlers who are not aware that
 GWT has different rules than all other Google3 code. Moving code from
 Google3 to GWT requires re-sorting.


Why is that any different than any other third-party product?  Plus one of
the first rules is to follow the style of the code around your change,
regardless of whether that matches the style guide.


 If you change the name of a function, you need to move the code to a new
 random location and on the review you can't see what really changed in the
 function or even if it has changed. I don't care much about internal stuff
 much but seeing an API in alphabetical order is as good as any other random
 order for the end users of the SDK.
 These are the similar reasons why none of other high profile libs (Guava,
 Guice, Dagger and JDK itself) enforce such rules.


I still prefer the way it is, and I think having one order (I don't
particularly care what that is) that is deterministic is better than
anything else, because the others always devolve into random orders as
people don't bother trying to organize it since it is already disorganized,
and it isn't brought up in reviews because there isn't any definitive
answer anyway.  Likewise for formatting rules.

-- 
John A. Tamplin

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT 
Contributors group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




Re: [gwt-contrib] GWT Coding Style Update

2013-05-30 Thread Matthew Dempsky
On Thu, May 30, 2013 at 10:39 PM, John A. Tamplin j...@jaet.org wrote:

 Why was it stopped being enforced?  It used to run :ant_tests (IIRC) that
 ran checkstyle, and TAP definitely caught style violations.


checkstyle is still run, and it's once of the checks that Jenkins runs
automatically on new submissions because style violations were so common on
Gerrit too.

I believe we relaxed the style rules a few months back though:
https://gwt-review.googlesource.com/1051

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT 
Contributors group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.