[gwt-contrib] Change in gwt[master]: Fix non-final field initializers running before the super cstr.
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
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
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...
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...
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
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...
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)
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...
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...
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...
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...
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.
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
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
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...
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
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)
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
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
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)
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)
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
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...
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
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
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...
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...
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
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.
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
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
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
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.
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.
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.
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.
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
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.
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...
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.
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...
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.
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...
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)
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
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
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
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
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
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
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
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
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.