[gwt-contrib] Re: Fixes issue 6653: Activity interface should use the new EventBus (issue1786804)

2012-07-20 Thread t . broyer
If we go that road, we should probably remove c.g.g.event.shared.EventBus completely. There's no reason to force people to move to the web.bindery version in some places (activities) but not others. This is something the Steering Committee should discuss (along with removing other deprecated

[gwt-contrib] Re: java.lang.String: adding support for certain regexp embedded java flags (issue1784803)

2012-07-19 Thread t . broyer
I didn't check, but when is StringJreTest launched? Shouldn't it be part of a TestSuite? (also, please CC google-web-toolkit-contributors@googlegroups.com in your reviews; I think you have to join the group first, but you should be prepared to receive answers there too anyway).

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

2012-07-19 Thread t . broyer
LGTM http://gwt-code-reviews.appspot.com/1785803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: @RequestFor should support interfaces, Fix issue 7509 (issue1764804)

2012-07-16 Thread t . broyer
/1764804/diff/9007/user/src/com/google/web/bindery/requestfactory/server/ResolverServiceLayer.java#newcode65 user/src/com/google/web/bindery/requestfactory/server/ResolverServiceLayer.java:65: public T Class? extends T resolveClientType(Class? domainClass, ClassT clientClass) { There's no reason

[gwt-contrib] Re: @RequestFor should support interfaces, Fix issue 7509 (issue1764804)

2012-07-16 Thread t . broyer
user/src/com/google/web/bindery/requestfactory/server/ResolverServiceLayer.java:65: private T Class? extends T resolveClientType(Class? domainClass, ClassT clientClass) { We talked in the Steering Committee about relaxing the rule, but for now private methods have to go after public, protected

[gwt-contrib] IE8 32k data:url bugfix (issue1778803)

2012-07-14 Thread t . broyer
http://gwt-code-reviews.appspot.com/1778803/diff/1/user/src/com/google/gwt/resources/Resources.gwt.xml File user/src/com/google/gwt/resources/Resources.gwt.xml (right): http://gwt-code-reviews.appspot.com/1778803/diff/1/user/src/com/google/gwt/resources/Resources.gwt.xml#newcode76

[gwt-contrib] Re: IE8 32k data:url bugfix (issue1778803)

2012-07-14 Thread t . broyer
http://gwt-code-reviews.appspot.com/1778803/diff/1/user/src/com/google/gwt/resources/rebind/context/InlineClientBundleGeneratorIE8.java File user/src/com/google/gwt/resources/rebind/context/InlineClientBundleGeneratorIE8.java (right):

[gwt-contrib] Re: @RequestFor should support interfaces, Fix issue 7509 (issue1764804)

2012-07-11 Thread t . broyer
Please add unit-tests (even better if you test edge-cases). Also, John no longer works at Google (or has he moved to other offices?), and is probably not the best fit for a review on RequestFactory. Finally, could you please base your patch on trunk rather than releases/2.5 (not really an issue

[gwt-contrib] Remove DOCTYPEs from all our modules. (issue1772803)

2012-07-09 Thread t . broyer
Reviewers: rdayal, skybrian, Message: A small one for your last day guys. Description: Remove DOCTYPEs from all our modules. Issue 5605 Please review this at https://gwt-code-reviews.appspot.com/1772803/ Affected files: M user/src/com/google/gwt/activity/Activity.gwt.xml M

[gwt-contrib] Re: Remove DOCTYPEs from all our modules. (issue1772803)

2012-07-09 Thread t . broyer
On 2012/07/09 15:37:43, skybrian wrote: On 2012/07/09 15:30:02, tbroyer wrote: A small one for your last day guys. Seems fine to me. But I wonder if we're just hiding some underlying issue with the XML parser we're using. There's no particular reason why development mode should be

[gwt-contrib] Re: Remove DOCTYPEs from all our modules. (issue1772803)

2012-07-09 Thread t . broyer
On 2012/07/09 15:39:57, tbroyer wrote: On 2012/07/09 15:37:43, skybrian wrote: On 2012/07/09 15:30:02, tbroyer wrote: A small one for your last day guys. Seems fine to me. But I wonder if we're just hiding some underlying issue with the XML parser we're using. There's no particular

[gwt-contrib] Include sources in gwt-codeserver.jar (issue1769803)

2012-06-29 Thread t . broyer
Reviewers: skybrian, rchandia, Description: Include sources in gwt-codeserver.jar This should fix the Maven deployment to Central, which requires sources (at least now there's a gwt-codeserver-${gwt.version}-sources.jar artifact generated and deployed). Issue: 7471 Please review this at

[gwt-contrib] Issue 7062: documentation uses @DefaultText instead of @DefaultMessage (issue1766803)

2012-06-28 Thread t . broyer
Reviewers: jat, Description: Issue 7062: documentation uses @DefaultText instead of @DefaultMessage Please review this at https://gwt-code-reviews.appspot.com/1766803/ Affected files: M user/src/com/google/gwt/i18n/client/Messages.java M

[gwt-contrib] Re: Update to 1.6 for javac source/target. (issue1753803)

2012-06-21 Thread t . broyer
LGTM http://gwt-code-reviews.appspot.com/1753803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix requestfactory test errors, and compilation failures in dev/compile.test resulting from the ... (issue1743808)

2012-06-20 Thread t . broyer
LGTM http://gwt-code-reviews.appspot.com/1743808/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Integrate Elemental to the build and deploy as Maven artifact (issue1727808)

2012-06-15 Thread t . broyer
http://gwt-code-reviews.appspot.com/1727808/diff/8001/build.xml File build.xml (right): http://gwt-code-reviews.appspot.com/1727808/diff/8001/build.xml#newcode41 build.xml:41: call-subproject subproject=elemental subtarget=build / On 2012/06/15 00:34:59, skybrian wrote: I asked Ray about this.

[gwt-contrib] Issue 7428: include the StreamHtmlParser within gwt-servlet.jar (issue1741803)

2012-06-15 Thread t . broyer
Reviewers: cromwellian, skybrian, Message: Another dependency packaging issue. Description: Issue 7428: include the StreamHtmlParser within gwt-servlet.jar Also moves from gwt-dev.jar to gwt-user.jar, where it should have always been. Please review this at

[gwt-contrib] Re: Integrate Elemental to the build and deploy as Maven artifact (issue1727808)

2012-06-15 Thread t . broyer
On 2012/06/15 15:32:26, rdayal wrote: On 2012/06/15 02:39:33, skybrian wrote: I verified that the ant build and google builds both work. The ant build takes about 9 minutes with this change, versus about 5:30 before. Also, elemental gets a full rebuild each time so a no-op build now takes

[gwt-contrib] Re: Integrate Elemental to the build and deploy as Maven artifact (issue1727808)

2012-06-15 Thread t . broyer
https://gwt-code-reviews.appspot.com/1727808/diff/16001/distro-source/build.xml File distro-source/build.xml (right): https://gwt-code-reviews.appspot.com/1727808/diff/16001/distro-source/build.xml#newcode39 distro-source/build.xml:39: zipfileset file=${gwt.build.lib}/gwt-elemental.jar

[gwt-contrib] Re: Make RfValidator Java7 Compat. (issue1743803)

2012-06-15 Thread t . broyer
LGTM http://gwt-code-reviews.appspot.com/1743803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Include json-1.5.jar in gwt-dev.jar. JSON is needed by the Closure (issue1732804)

2012-06-14 Thread t . broyer
LGTM. I'll make another CL updating the Maven artifacts. http://gwt-code-reviews.appspot.com/1732804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Include json-1.5.jar in gwt-dev.jar. JSON is needed by the Closure (issue1732804)

2012-06-14 Thread t . broyer
On 2012/06/14 07:17:21, tbroyer wrote: I'll make another CL updating the Maven artifacts. http://gwt-code-reviews.appspot.com/1732804/ http://gwt-code-reviews.appspot.com/1732804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Include json-1.5.jar in gwt-dev.jar. JSON is needed by the Closure (issue1732804)

2012-06-14 Thread t . broyer
On 2012/06/14 07:25:50, tbroyer wrote: On 2012/06/14 07:17:21, tbroyer wrote: I'll make another CL updating the Maven artifacts. http://gwt-code-reviews.appspot.com/1732804/ Oops, sorry, wrong link: https://gwt-code-reviews.appspot.com/1737805/

[gwt-contrib] Update Maven deployment re. JSON dependency. (issue1737805)

2012-06-14 Thread t . broyer
Reviewers: cromwellian, skybrian, Message: This is a follow up to http://gwt-code-reviews.appspot.com/1732804/ It unbundles org/json from gwt-dev before deploying to Maven Central, and adds it as a dependency instead (as we did for requestfactory-*). org.json is also added as a dependency to

[gwt-contrib] Re: Bundle org.json in gwt-dev and javax.validation into gwt-user, unbundle from requestfactory-* (issue1731804)

2012-06-14 Thread t . broyer
On 2012/06/14 01:20:58, skybrian wrote: To fix the compiler and close issue 7397, we only need to add json (of any version) to gwt-dev.jar. So I think I'm going to commit a tiny change that just does that. We can defer the other stuff until we have a better plan. OK let's do that. Baby

[gwt-contrib] Re: Integrate Elemental to the build and deploy as Maven artifact (issue1727808)

2012-06-14 Thread t . broyer
Updated the CL with some fixes to the Ant build: - failed in the absence of a CC environment variable - tried to copy to elemental/java where as the source is in elemental/src; instead of copying to src, I instead kept the generated source separate and simply added elemented/idl/generated/src

[gwt-contrib] Re: Fix SimpleViolation so that ConstraintViolations with paths that don't map to any editors are st... (issue1727807)

2012-06-13 Thread t . broyer
I'll leave the final word to Brian. Some minor comments below, but otherwise looks good. http://gwt-code-reviews.appspot.com/1727807/diff/14001/user/src/com/google/gwt/editor/client/impl/SimpleViolation.java File user/src/com/google/gwt/editor/client/impl/SimpleViolation.java (right):

[gwt-contrib] Re: Fix SimpleViolation so that ConstraintViolations with paths that don't map to any editors are st... (issue1727807)

2012-06-13 Thread t . broyer
Sorry for not having spotted it in the previous patch set: the new 'private static' methods are not ordered alphabetically (yes, they're in logical order, but the coding style for GWT mandates alphabetical ordering...) Otherwise good (I'd still have splitted the for() loop with the if/else

[gwt-contrib] Re: Elemental + build scripts initial import. (issue1728806)

2012-06-13 Thread t . broyer
Only looked at elemental.json.* so far. Looks like the Js implementations haven't been used much in DevMode (which is not really surprising given the super-source implementation of element.json.Json, which is @GwtScriptOnly, so only an explicit 'new JsJsonFactory()', or casting a JSO to a

[gwt-contrib] Integrate Elemental to the build and deploy as Maven artifact (issue1727808)

2012-06-13 Thread t . broyer
Reviewers: skybrian, cromwellian, https://gwt-code-reviews.appspot.com/1727808/diff/1/build.xml File build.xml (left): https://gwt-code-reviews.appspot.com/1727808/diff/1/build.xml#oldcode57 build.xml:57: gwt.ant dir=dev/codeserver / See

[gwt-contrib] Re: Bundle org.json in gwt-dev and javax.validation into gwt-user, unbundle from requestfactory-* (issue1731804)

2012-06-13 Thread t . broyer
I've removed the changes concerning javax.validation, so it's still distributed as a separate JAR. org.json is now bundled in gwt-dev, no longer bundled into requestfactory-* JARs, and distributed as a separate JAR in the SDK (so that requestfactory users have the choice to user

[gwt-contrib] Re: Update POM versions to 2.5.0-rc1 (issue1734804)

2012-06-13 Thread t . broyer
LGTM http://gwt-code-reviews.appspot.com/1734804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Elemental + build scripts initial import. (issue1728806)

2012-06-13 Thread t . broyer
On 2012/06/13 23:33:46, cromwellian wrote: Thomas, I'm a little knee deep in I/O slide stuff at the moment, so maybe you can sanity check my thinking here. Let me describe what used to be happening in the Json stuff and what I was trying to change it to before 2.5, but probably didn't

[gwt-contrib] Re: Bundle org.json in gwt-dev and javax.validation into gwt-user, unbundle from requestfactory-* (issue1731804)

2012-06-13 Thread t . broyer
On 2012/06/13 21:45:00, skybrian wrote: Okay, seems fine for now. I'm going to commit this. FYI, I just removed all occurrences of json-1.5.jar, so we consistently use json.jar everywhere. https://gwt-code-reviews.appspot.com/1731804/ --

[gwt-contrib] Re: Fix SimpleViolation so that ConstraintViolations with paths that don't map to any editors are st... (issue1727807)

2012-06-12 Thread t . broyer
Almost OK! http://gwt-code-reviews.appspot.com/1727807/diff/7/user/src/com/google/gwt/editor/client/impl/SimpleViolation.java File user/src/com/google/gwt/editor/client/impl/SimpleViolation.java (right):

[gwt-contrib] Re: Fix SimpleViolation so that ConstraintViolations with paths that don't map to any editors are st... (issue1727807)

2012-06-11 Thread t . broyer
http://gwt-code-reviews.appspot.com/1727807/diff/1/user/src/com/google/gwt/editor/client/impl/SimpleViolation.java File user/src/com/google/gwt/editor/client/impl/SimpleViolation.java (right):

[gwt-contrib] Re: Fix SimpleViolation so that ConstraintViolations with paths that don't map to any editors are st... (issue1727807)

2012-06-11 Thread t . broyer
On 2012/06/11 14:30:14, sameb wrote: Re: Using the immediate parent's editor instead of the root editor, I'm not sure about the change. I can get behind using non-LeafValueEditor parent editors (ie, publish to 'address' if 'address.bogus' is the path), but I don't think it makes sense to

[gwt-contrib] Re: Fix SimpleViolation so that ConstraintViolations with paths that don't map to any editors are st... (issue1727807)

2012-06-11 Thread t . broyer
I was thinking more simply about wrapping the whole getDelegatesByPath/getEditorByPath/fallback, starting at line 131, into a loop: boolean found = false; do { // Find the leaf editor's delegate. ListAbstractEditorDelegate?, ? leafDelegates = delegateMap.getDelegatesByPath(absolutePath);

[gwt-contrib] ValidationTool doesn't flag abstract classes as non-default-instantiable (issue1729805)

2012-06-10 Thread t . broyer
Reviewers: skybrian, rdayal, https://gwt-code-reviews.appspot.com/1729805/diff/1/build.xml File build.xml (left): https://gwt-code-reviews.appspot.com/1729805/diff/1/build.xml#oldcode57 build.xml:57: gwt.ant dir=dev/codeserver / ant testrf failed because it tried to call compile.tests in

[gwt-contrib] Re: Add new CSS FunctionValue type as a first-class representation of value functions, hook it up in... (issue1735804)

2012-06-09 Thread t . broyer
LGTM (provided you address the comments below) http://gwt-code-reviews.appspot.com/1735804/diff/1/user/src/com/google/gwt/resources/css/GenerateCssAst.java File user/src/com/google/gwt/resources/css/GenerateCssAst.java (right):

[gwt-contrib] Re: Add GWT CSS compiler support for RTL flipping of many CSS3 properties (border-image, border-radi... (issue1727803)

2012-06-09 Thread t . broyer
LGTM http://gwt-code-reviews.appspot.com/1727803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Customizable Place delimiter (issue1710803)

2012-06-09 Thread t . broyer
Thinking about it, the 'separator' in WithTokenizers makes it mandatory to provide a 'value' for the annotation, even if you're only using PlaceTokenizers from a factory (would @WithTokenizers(value={}, separator=/) even work?) The configuration-property wouldn't have that drawback. Also,

[gwt-contrib] Re: Bundle org.json in gwt-dev and javax.validation into gwt-user, unbundle from requestfactory-* (issue1731804)

2012-06-08 Thread t . broyer
On 2012/06/08 18:02:44, skybrian wrote: Moving org.json into gwt-dev makes sense because the compiler actually uses it. I'd like to see it repackaged, but perhaps we can wait until someone complains? I'm not sure about validation. Many people don't use it and I sorta think GWT's

[gwt-contrib] Re: Add GWT CSS compiler support for RTL flipping of many CSS3 properties (border-image, border-radi... (issue1727803)

2012-06-07 Thread t . broyer
On 2012/06/07 07:39:38, ehwang wrote: Thanks for the review! For the vendor prefixes, I ended up creating a separate helper class, VendorPrefixes. It doesn't keep a master list of prefixes. I decided that was more future-proof, and the -foo- syntax is specifically reserved by the W3C for

[gwt-contrib] Re: Add Map support to RequestFactory (issue 6132056)

2012-06-07 Thread t . broyer
On 2012/06/07 08:20:34, james.horsley wrote: Rajeev, get a chance to look at this yet? We're cutting out GWT 2.5 soon; we decided to postpone this change to the next version. BTW, sorry for being picky, could you please: - make your patch at the trunk/ level (rather than trunk/user/) - post

[gwt-contrib] Re: Add Map support to RequestFactory (issue 6132056)

2012-06-07 Thread t . broyer
On 2012/06/07 09:22:24, james.horsley wrote: Thanks for all your help with the patch Thomas. Sad to hear it won't make it into GWT 2.5 as we were really looking to use RF with Maps without having to use a patched version of GWT built from source internally. We're planning a 2.5.1 soon

[gwt-contrib] Integrate SuperDevMode to the build and deploy as Maven artifact (issue1734803)

2012-06-06 Thread t . broyer
Reviewers: skybrian, cromwellian, Description: Integrate SuperDevMode to the build and deploy as Maven artifact Please review this at https://gwt-code-reviews.appspot.com/1734803/ Affected files: M build.xml M distro-source/build.xml M maven/lib-gwt.sh A

[gwt-contrib] Re: Move Super Dev Mode to the open source repository. (issue1727804)

2012-06-05 Thread t . broyer
Most of my comments should be seen as TODOs, as I really don't want to delay landing: this is highly experimental anyway, and we know it'll need some more work. http://gwt-code-reviews.appspot.com/1727804/diff/1/dev/codeserver/java/com/google/gwt/dev/codeserver/AppSpace.java File

[gwt-contrib] Re: Add GWT CSS compiler support for RTL flipping of many CSS3 properties (border-image, border-radi... (issue1727803)

2012-06-05 Thread t . broyer
http://gwt-code-reviews.appspot.com/1727803/diff/1/user/src/com/google/gwt/resources/css/RtlVisitor.java File user/src/com/google/gwt/resources/css/RtlVisitor.java (right): http://gwt-code-reviews.appspot.com/1727803/diff/1/user/src/com/google/gwt/resources/css/RtlVisitor.java#newcode43

[gwt-contrib] Re: Adding the GWT 2.4_2.5 API Checker configuration file. Many API changes were added to the 2.3_2... (issue1721803)

2012-05-31 Thread t . broyer
On 2012/05/30 16:35:33, jlabanca wrote: That would involve creating 2.3-modified jars, syncing to the 2.4 release, and running API Checker, just to update a file that is no longer used. I'll leave it for the next guy to worry about. I meant reverting (svn revert or git revert) some of the

[gwt-contrib] Re: Fix issue 6710: entities referenced from list of value proxies (issue1646803)

2012-05-31 Thread t . broyer
https://gwt-code-reviews.appspot.com/1646803/diff/14003/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java File user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java (right):

[gwt-contrib] Re: Issue 7038: CompositeEditor and ListEditor optimizations (issue1664803)

2012-05-31 Thread t . broyer
On 2012/05/31 00:18:43, skybrian wrote: I got this feedback from someone who requested a rollback but decided to work around it instead. I wonder how widespread this sort of thing is? Our use of the editor/driver stuff assumed the editors would be rebuilt when driver.edit(T) was invoked

[gwt-contrib] Re: Fix for issue 5952: RequestContext#isChanged. (issue1601806)

2012-05-31 Thread t . broyer
On 2012/05/31 01:56:32, skybrian wrote: LGTM (assuming tests still pass) Because I must confess I didn't run them on the last few patch-sets, I just ran ant clean testrf and then the RequestFactorySuite in both prod and dev mode from within Eclipse, and I confirm everything's OK.

[gwt-contrib] Re: Fix issue 6710: entities referenced from list of value proxies (issue1646803)

2012-05-31 Thread t . broyer
https://gwt-code-reviews.appspot.com/1646803/diff/21002/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java File user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java (right):

RequestFactory - how to prevent batched requests after constraint violation

2012-05-30 Thread Bill T
I am getting an IllegalArgumentException, Attempting to edit an EntityProxy previously edited by another RequestContext. This is a result of getting constraint violations and automatically batching different Request Factory requests within the RequestContext. I am using GWT 2.4. My

[gwt-contrib] Re: DefaultProxyStore violated ProxyStore#nextId contract. (issue1622803)

2012-05-30 Thread t . broyer
On 2012/05/29 22:07:38, skybrian wrote: Actually, are we working on dead code? Where is serialize/deserialize of a DefaultProxyStore used? I'm looking for the code that calls encode() and I can't seem to find it. I traced back usages of ProxyStore to RequestFactory.getSerializer() but can't

[gwt-contrib] Re: DefaultProxyStore violated ProxyStore#nextId contract. (issue1622803)

2012-05-30 Thread t . broyer
I just updated the CL with a very minor fix: in encode() I saved nextId + 1 (so as to never store a '0' and be able to detect a payload serialized by the old, broken DefaultProxyStore), but never reverted the + 1 in the constructor, resulting in skipping a value each time to serialize/deserialize

[gwt-contrib] Re: Generated EditorContext class names take actual parameterization into account. (issue1352806)

2012-05-30 Thread t . broyer
On 2012/05/30 01:39:13, skybrian wrote: On 2012/05/26 15:20:15, tbroyer wrote: On 2012/05/25 19:35:47, skybrian wrote: Having two levels of class nesting is rather confusing. Could you move the Driver and other 4 inner classes up a level? Done. Do you think the

[gwt-contrib] Re: Fix issue 6710: entities referenced from list of value proxies (issue1646803)

2012-05-30 Thread t . broyer
https://gwt-code-reviews.appspot.com/1646803/diff/1/user/src/com/google/web/bindery/requestfactory/server/Resolver.java File user/src/com/google/web/bindery/requestfactory/server/Resolver.java (right):

[gwt-contrib] Re: Adding the GWT 2.4_2.5 API Checker configuration file. Many API changes were added to the 2.3_2... (issue1721803)

2012-05-30 Thread t . broyer
On 2012/05/30 15:49:24, jlabanca wrote: We don't delete the old files, we just add a new file. But maybe we should revert the changes made to the 23_24 file after the 2.4 release was cut out, no? (re. Many API changes were added to the 2.3_2.4 file because we never created the 2.4_2.5 config

[gwt-contrib] Re: Fix issue 6710: entities referenced from list of value proxies (issue1646803)

2012-05-27 Thread t . broyer
https://gwt-code-reviews.appspot.com/1646803/diff/1/user/src/com/google/web/bindery/requestfactory/server/Resolver.java File user/src/com/google/web/bindery/requestfactory/server/Resolver.java (right):

[gwt-contrib] Re: Issue 7038: CompositeEditor and ListEditor optimizations (issue1664803)

2012-05-26 Thread t . broyer
On 2012/05/25 22:53:18, tbroyer wrote: On 2012/05/25 17:58:21, skybrian wrote: In getEditors: The returned list will be live until the next call to setValue() and shouldn't be used after that. [...] Yeah, #3 is out. Option #2 would make sense if we expected that complaining early

[gwt-contrib] Re: Generated EditorContext class names take actual parameterization into account. (issue1352806)

2012-05-26 Thread t . broyer
http://gwt-code-reviews.appspot.com/1352806/diff/2001/user/test/com/google/gwt/editor/client/SimpleBeanEditorTest.java File user/test/com/google/gwt/editor/client/SimpleBeanEditorTest.java (right):

[gwt-contrib] Re: DefaultProxyStore violated ProxyStore#nextId contract. (issue1622803)

2012-05-26 Thread t . broyer
http://gwt-code-reviews.appspot.com/1622803/diff/6001/user/src/com/google/web/bindery/requestfactory/shared/DefaultProxyStore.java File user/src/com/google/web/bindery/requestfactory/shared/DefaultProxyStore.java (right):

[gwt-contrib] Re: Fix for issue 5952: RequestContext#isChanged. (issue1601806)

2012-05-25 Thread t . broyer
On 2012/05/25 21:43:28, sjostrand.jonas wrote: When a proxy is edit()ed not only sub ENTITY proxies but also sub VALUE proxies are automatically edit()ed and considered as changed by the auto bean diff. Can this be fixed by the same technique? I tried the patch and it didn't solve the

[gwt-contrib] ExternalTextResourceGenerator is locale/machine-dependent. (issue1716804)

2012-05-25 Thread t . broyer
Reviewers: , Message: Looking for a reviewer on this. Note there are other uses of getBytes() without a specified encoding: - in XsrfProtectedServiceServlet and XsrfTokenServiceServlet (though it's not that important, as it's about creating and validating a token; it could be an issue

[gwt-contrib] Re: Issue 7038: CompositeEditor and ListEditor optimizations (issue1664803)

2012-05-25 Thread t . broyer
On 2012/05/25 17:58:21, skybrian wrote: In getEditors: The returned list will be live until the next call to setValue() and shouldn't be used after that. [...] Yeah, #3 is out. Option #2 would make sense if we expected that complaining early means that most people would fix the problem

[gwt-contrib] Re: Fix for issue 5952: RequestContext#isChanged. (issue1601806)

2012-05-25 Thread t . broyer
http://gwt-code-reviews.appspot.com/1601806/diff/45014/user/test/com/google/web/bindery/requestfactory/server/SimpleBar.java File user/test/com/google/web/bindery/requestfactory/server/SimpleBar.java (right):

[gwt-contrib] Re: Fix issue 6959. (issue1587803)

2012-05-24 Thread t . broyer
Ping http://gwt-code-reviews.appspot.com/1587803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Generated EditorContext class names take actual parameterization into account. (issue1352806)

2012-05-24 Thread t . broyer
Ping http://gwt-code-reviews.appspot.com/1352806/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Addresses a couple of my concerns re. RF validation. (issue1577806)

2012-05-24 Thread t . broyer
Ping (sorry for the spam, these are important issues that we should really try to fix in 2.5, so I'm just making sure they're not forgotten) http://gwt-code-reviews.appspot.com/1577806/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fixing a Chrome animation bug where animations never stop running. The timestamp that webkitReq... (issue1702803)

2012-05-23 Thread t . broyer
Looks like we should re-open this CL: http://updates.html5rocks.com/2012/05/requestAnimationFrame-API-now-with-sub-millisecond-precision We might also want to revisit the Mozilla implementation with a similar change to make sure it won't break in a future version of Firefox. (and also note that,

[gwt-contrib] Re: Fix for issue 5952: RequestContext#isChanged. (issue1601806)

2012-05-23 Thread t . broyer
http://gwt-code-reviews.appspot.com/1601806/diff/40003/user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java File user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java (right):

[gwt-contrib] Re: Add white-space support to Style (issue1714803)

2012-05-23 Thread t . broyer
LGTM And now we have to find a committer to get this in: John maybe? http://gwt-code-reviews.appspot.com/1714803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fixing a Chrome animation bug where animations never stop running. The timestamp that webkitReq... (issue1702803)

2012-05-23 Thread t . broyer
LGTM http://gwt-code-reviews.appspot.com/1702803/diff/5001/user/src/com/google/gwt/animation/client/AnimationSchedulerImplMozilla.java File user/src/com/google/gwt/animation/client/AnimationSchedulerImplMozilla.java (right):

[gwt-contrib] Re: Fix for issue 5952: RequestContext#isChanged. (issue1601806)

2012-05-23 Thread t . broyer
http://gwt-code-reviews.appspot.com/1601806/diff/45014/user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java File user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java (right):

[gwt-contrib] Re: Add Integer.TYPE, etc. (issue1528806)

2012-05-22 Thread t . broyer
http://gwt-code-reviews.appspot.com/1528806/diff/8001/user/super/com/google/gwt/emul/java/lang/Double.java File user/super/com/google/gwt/emul/java/lang/Double.java (right): http://gwt-code-reviews.appspot.com/1528806/diff/8001/user/super/com/google/gwt/emul/java/lang/Double.java#newcode36

[gwt-contrib] Revert GWT support for Chrome Frame (issue1713803)

2012-05-22 Thread t . broyer
Reviewers: cromwellian, Description: Revert GWT support for Chrome Frame This reverts commit r10250. Fixes issue 6665. Please review this at http://gwt-code-reviews.appspot.com/1713803/ Affected files: M user/src/com/google/gwt/useragent/rebind/UserAgentPropertyGenerator.java Index:

[gwt-contrib] Re: Removing uses of deprecated Tree code in GWT showcase sample and TreeExample. (issue1712804)

2012-05-22 Thread t . broyer
http://gwt-code-reviews.appspot.com/1712804/diff/1/samples/showcase/src/com/google/gwt/sample/showcase/client/content/lists/CwStackPanel.java File samples/showcase/src/com/google/gwt/sample/showcase/client/content/lists/CwStackPanel.java (right):

[gwt-contrib] Re: Fix issue 5658: PlaceHistoryGeneratorContext now examines PlaceTokenizers' hierarchy. (issue1674804)

2012-05-19 Thread t . broyer
https://gwt-code-reviews.appspot.com/1674804/diff/1/user/src/com/google/gwt/place/rebind/PlaceHistoryGeneratorContext.java File user/src/com/google/gwt/place/rebind/PlaceHistoryGeneratorContext.java (right):

[gwt-contrib] Re: Fix issue 5658: PlaceHistoryGeneratorContext now examines PlaceTokenizers' hierarchy. (issue1674804)

2012-05-17 Thread t . broyer
https://gwt-code-reviews.appspot.com/1674804/diff/1/user/src/com/google/gwt/place/rebind/PlaceHistoryGeneratorContext.java File user/src/com/google/gwt/place/rebind/PlaceHistoryGeneratorContext.java (right):

[gwt-contrib] Re: Fix for issue 5952: RequestContext#isChanged. (issue1601806)

2012-05-17 Thread t . broyer
On 2012/05/15 19:31:41, skybrian wrote: More naive questions: It seems like instead of having EntityProxy.equals() work in two different ways, we should inline AutoBeanUtils.diff() and simplify it to do exactly what we want for this case, without relying on Object.equals() if possible? We

[gwt-contrib] Re: Issue 7038: CompositeEditor and ListEditor optimizations (issue1664803)

2012-05-17 Thread t . broyer
Hi Brian, I just saw it was rolled back. Patchset #2 fixes the NPE (that was rather obvious actually). Should I also add a test? https://gwt-code-reviews.appspot.com/1664803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Issue 7038: CompositeEditor and ListEditor optimizations (issue1664803)

2012-05-17 Thread t . broyer
On 2012/05/17 21:38:03, skybrian wrote: Oops, I had started an email about the revert but apparently never sent it. Sorry about that! No problem! I might need some background about the editor framework...

[gwt-contrib] Re: Fix for issue 5952: RequestContext#isChanged. (issue1601806)

2012-05-17 Thread t . broyer
On 2012/05/17 22:52:23, skybrian wrote: Okay, yeah, a TODO is fine as it looks kind of difficult. OK, I'll create an issue, add a TODO referencing it and update the CL. (one of the difficulties is that the only way to get at properties of an AutoBean is to visit it, so I think we'd still need

[gwt-contrib] Re: Fix for issue 5952: RequestContext#isChanged. (issue1601806)

2012-05-15 Thread t . broyer
On 2012/05/14 22:34:17, skybrian wrote: I tried reviewing this but I'm missing some things. When can an EntityProxy have a null RequestContext? Aren't all entities created from a RequestContext? Immutable EntityProxies don't have a context (see AbstractRequestContext#makeImmutable); this

[gwt-contrib] Re: Fixes issue 4301: ensures all strings generated by GWT are JS string values. (issue1623803)

2012-05-15 Thread t . broyer
http://gwt-code-reviews.appspot.com/1623803/diff/9001/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java File dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java (right):

[gwt-contrib] Re: DefaultProxyStore violated ProxyStore#nextId contract. (issue1622803)

2012-05-15 Thread t . broyer
On 2012/05/14 19:09:11, rdayal wrote: Had to roll this back. These are the test failures that I'm seeing: 1) testValueObjectCreateSetRetrieveUpdateViaList(com.google.web.bindery.requestfactory.gwt.client.RequestFactoryTest)java.lang.RuntimeException: Remote test failed at 127.0.0.1 /

[gwt-contrib] Re: Fixes issue 4301: ensures all strings generated by GWT are JS string values. (issue1623803)

2012-05-15 Thread t . broyer
http://gwt-code-reviews.appspot.com/1623803/diff/25001/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java File dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java (right):

[gwt-contrib] Re: Resubmission for internal commit. Thanks tbroyer! (issue1711803)

2012-05-15 Thread t . broyer
Ah, sorry, there was yet another update (that one's broken according to scottb) http://gwt-code-reviews.appspot.com/1711803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix for issue 5952: RequestContext#isChanged. (issue1601806)

2012-05-14 Thread t . broyer
I've fix the test (I thought it was environmental, but it happened to be a 2-char-long mistake: .to() instead of .fire(): the request was never sent, so finishTestAndReset was never called (from the response)). That wasn't an issue in JRE tests, where everything runs synchronously; so the

[gwt-contrib] Re: Fixes issue 4301: ensures all strings generated by GWT are JS string values. (issue1623803)

2012-05-14 Thread t . broyer
Would be great to get it into 2.5 ;-) http://gwt-code-reviews.appspot.com/1623803/diff/9001/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java File dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java (right):

[gwt-contrib] Re: Customizable Place delimiter (issue1710803)

2012-05-12 Thread t . broyer
Could you please fix the Base URL (http://google-web-toolkit.googlecode.com/svn/trunk/user/src/com/google/gwt/) and/or, much better, make the patch from the root of the repository (trunk/) Right now, the side-by-side diffs are not viewable and I cannot even send my comments (on the unified

[gwt-contrib] Re: Cast return value from JsDate's various setter methods to be (issue1704804)

2012-05-11 Thread t . broyer
On 2012/05/10 21:57:48, jat wrote: Are you sure this actually results in a usable and correct value? Since the spec doesn't say what is returned On the contrary, the spec (ECMA-262, both 3rd and 5th editions, even though worded very differently) is clear that the return value should be a

[gwt-contrib] Re: Issue 7231: Speed-up SimpleRequestProcessor wrt ValueProxies (issue1660803)

2012-05-10 Thread t . broyer
RequestFactory is indeed rather opaque, and *very* convoluted. More details below. https://gwt-code-reviews.appspot.com/1660803/diff/1/user/src/com/google/web/bindery/requestfactory/server/SimpleRequestProcessor.java File

[gwt-contrib] Re: Rendering a blank space in an empty EditTextCell to force the element to have a height. Otherwi... (issue1708803)

2012-05-10 Thread t . broyer
Thinking out loud... http://gwt-code-reviews.appspot.com/1708803/diff/1/user/src/com/google/gwt/cell/client/EditTextCell.java File user/src/com/google/gwt/cell/client/EditTextCell.java (right):

[gwt-contrib] Re: Issue 7231: Speed-up SimpleRequestProcessor wrt ValueProxies (issue1660803)

2012-05-10 Thread t . broyer
https://gwt-code-reviews.appspot.com/1660803/diff/1/user/src/com/google/web/bindery/requestfactory/server/SimpleRequestProcessor.java File user/src/com/google/web/bindery/requestfactory/server/SimpleRequestProcessor.java (right):

[gwt-contrib] Re: Fix for issue 5952: RequestContext#isChanged. (issue1601806)

2012-05-04 Thread t . broyer
On 2012/05/04 22:05:23, rdayal wrote: Ok, more information - I'm only getting the failure when I run the entire RequestFactorySuite. Still digging, but can you tell me if you see a failure if you run the entire suite? Reproduced :-( I also have a timeout in

[gwt-contrib] Re: Fixing a Chrome animation bug where animations never stop running. The timestamp that webkitReq... (issue1702803)

2012-05-04 Thread t . broyer
Related: http://code.google.com/p/google-web-toolkit/issues/detail?id=7352 http://code.google.com/p/google-web-toolkit/issues/detail?id=7346 But it only happened in 20.0.1123.1 (and 20.0.1123.2 on Linux), it's no longer reproducible in 20.0.123.4. http://gwt-code-reviews.appspot.com/1702803/

[gwt-contrib] Re: Fix for issue 5952: RequestContext#isChanged. (issue1601806)

2012-05-04 Thread t . broyer
OK, found it: the issue in EditorTest is that the SimpleBar that the setUserName is applied to and the one in the SimpleFoo's barField are not the same instances! The instances are initialized in response to the finishTestAndReset call from RequestBatcherTest, which explains the differing

<    1   2   3   4   5   6   7   8   >