[gwt-contrib] Conditionalize rich text area tests for HTML Unit, use the initialize event for delays
Reviewers: jlabanca, jgw, Message: I re-wrote some of the tests to use the new initialize event instead of a timer to wait for the area to become available. Htmlunit has some issue with FF3 emulation, so I filtered out the InitializeHandler out for Htmlunit. I also found a bug in the IE implementation in that the initialize event was called back before the cached HTML data was transferred over into the iframe. Please review this at http://gwt-code-reviews.appspot.com/70801 Affected files: M user/src/com/google/gwt/user/client/ui/impl/RichTextAreaImplStandard.java M user/test/com/google/gwt/user/client/ui/RichTextAreaTest.java --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Conditionalize rich text area tests for HTML Unit, use the initialize event for delays
Thanks for the review. Committed as r6200 http://gwt-code-reviews.appspot.com/70801 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Remove -XstartOnFirstThread from generated ant oophm on Mac
http://gwt-code-reviews.appspot.com/62803/diff/1/2 File user/src/com/google/gwt/user/tools/project.ant.xmlsrc (right): http://gwt-code-reviews.appspot.com/62803/diff/1/2#newcode41 Line 41: !-- Additional arguments like -style PRETTY or -logLevel DEBUG -- Would be appropriate to remove it from here too? AFAIK, -XstartOnFirstThread only needed for SWT apps on a Mac. http://gwt-code-reviews.appspot.com/62803 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Remove -XstartOnFirstThread from generated ant oophm on Mac
LGTM http://gwt-code-reviews.appspot.com/62803/diff/1/2 File user/src/com/google/gwt/user/tools/project.ant.xmlsrc (right): http://gwt-code-reviews.appspot.com/62803/diff/1/2#newcode41 Line 41: !-- Additional arguments like -style PRETTY or -logLevel DEBUG -- On 2009/08/24 19:24:28, jat wrote: On 2009/08/24 19:05:01, zundel wrote: Would be appropriate to remove it from here too? AFAIK, -XstartOnFirstThread only needed for SWT apps on a Mac. It is possible to run the GUI treelogger for the compile, though I didn't see a way it could be done here without hacking it, and until SWT is ripped out it would need the flag. I suggest taking it out at the point we rip out SWT, and if we keep the ability to run the compiler in a treelogger it will be in Swing instead. ok http://gwt-code-reviews.appspot.com/62803 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Add Gears' BlobBuilder
Hi Thomas,
Thanks for the contribution.
I made a few suggestions and would like to see a unit test for these
classes along with the submission.
Try using 'zun...@google.com' as a reviewer.
-Eric.
http://3.latest.galgwt-reviews.appspot.com/41603/diff/1/2
File gears/src/com/google/gwt/gears/client/blobbuilder/BlobBuilder.java
(right):
http://3.latest.galgwt-reviews.appspot.com/41603/diff/1/2#newcode25
Line 25: public final class BlobBuilder extends JavaScriptObject {
Class needs javadoc. From the JS documentation:
A BlobBuilder is a helper object for creating Blobs. A Blob is an
immutable, read-only object, the same as a JavaScript string.
BlobBuilders are used to generate a Blob with new content. They are to
Java's StringBuilder as Gears Blobs are to Java's Strings.
http://3.latest.galgwt-reviews.appspot.com/41603/diff/1/2#newcode31
Line 31: public native void append(byte c) /*-{
These public methods need javadoc. We usually just copy as much as
possible from the gears JavaScript API:
Appends bytes to the Blob-in-progress.
http://3.latest.galgwt-reviews.appspot.com/41603/diff/1/2#newcode43
Line 43: public void append(byte[] bytes) {
I think you could wrap up append (byte c) and this one with a prototype
like:
append (byte... c)
I'm not sure what drawbacks there might be.
http://3.latest.galgwt-reviews.appspot.com/41603/diff/1/2#newcode51
Line 51: public void append(String[] strings) {
again, append(String... strings) could catch both methods
http://3.latest.galgwt-reviews.appspot.com/41603/diff/1/2#newcode67
Line 67: private native void append(JsArrayInteger bytes) /*-{
These three could all be a single method prototyped as
private native void append(JavaScriptObject jso)
http://3.latest.galgwt-reviews.appspot.com/41603
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] Re: Add Gears' BlobBuilder
http://3.latest.galgwt-reviews.appspot.com/41603/diff/1/2
File gears/src/com/google/gwt/gears/client/blobbuilder/BlobBuilder.java
(right):
http://3.latest.galgwt-reviews.appspot.com/41603/diff/1/2#newcode43
Line 43: public void append(byte[] bytes) {
On 2009/07/06 13:26:56, zundel wrote:
I think you could wrap up append (byte c) and this one with a
prototype like:
append (byte... c)
I'm not sure what drawbacks there might be.
I did some checking around and maybe this is not such a good idea. The
Java tutorial on varargs says: Generally speaking, you should not
overload a varargs method, or it will be difficult for programmers to
figure out which overloading gets called. I played around with the
ArrayHelper class writing a unit tests in a similar situation and found
that the problem isn't just with programmers, the compiler seems to get
confused as well!
http://3.latest.galgwt-reviews.appspot.com/41603
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] Re: Allow @def to be retrieved as a String from CssResource
I added a test to show that the @ClassName annotation works to
differentiate between an @def and a class name accessor.
http://gwt-code-reviews.appspot.com/50804/diff/1/5
File user/src/com/google/gwt/resources/css/ast/CssVisitor.java (right):
http://gwt-code-reviews.appspot.com/50804/diff/1/5#newcode188
Line 188: e.printStackTrace();
On 2009/06/30 21:00:45, bobv wrote:
Remove.
Done.
http://gwt-code-reviews.appspot.com/50804/diff/1/6
File user/src/com/google/gwt/resources/rg/CssResourceGenerator.java
(right):
http://gwt-code-reviews.appspot.com/50804/diff/1/6#newcode1739
Line 1739: if
(String.equals(toImplement.getReturnType().getSimpleSourceName())) {
On 2009/06/30 21:00:45, bobv wrote:
Use JClassType comparison for correctness. This would match
com.foo.String.
Done.
http://gwt-code-reviews.appspot.com/50804/diff/1/6#newcode1740
Line 1740: returnExpr = \ + def.getValues().get(0) + \;
On 2009/06/30 21:00:45, bobv wrote:
The value has to be escaped; see the Generator.escape().
Done.
http://gwt-code-reviews.appspot.com/50804/diff/1/6#newcode1790
Line 1790: // TODO(zundel): make conditional on Strict mode?
On 2009/06/30 21:00:45, bobv wrote:
This condition should always be an error.
Done.
http://gwt-code-reviews.appspot.com/50804/diff/1/4
File user/test/com/google/gwt/resources/client/test.css (right):
http://gwt-code-reviews.appspot.com/50804/diff/1/4#newcode24
Line 24:
On 2009/06/30 21:00:45, bobv wrote:
Revert.
Done.
http://gwt-code-reviews.appspot.com/50804
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] Allow @def to be retrieved as a String from CssResource
Reviewers: robertvawter_google.com, Description: I would like to be able to use @def to define color values in my .css file but also access them in my Java code. This change allows you to define an accessor in your CssResource implementation class that returns a String. Please review this at http://gwt-code-reviews.appspot.com/50804 Affected files: user/src/com/google/gwt/resources/client/CssResource.java user/src/com/google/gwt/resources/css/ast/CssVisitor.java user/src/com/google/gwt/resources/rg/CssResourceGenerator.java user/test/com/google/gwt/resources/client/CSSResourceTest.java user/test/com/google/gwt/resources/client/deftest.css user/test/com/google/gwt/resources/client/test.css --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: reworking ant targets to allow single-platform use
http://gwt-code-reviews.appspot.com/37803/diff/1/2
File build.xml (right):
http://gwt-code-reviews.appspot.com/37803/diff/1/2#newcode21
Line 21: target name=dist-one depends=buildonly, tools, samples,
doc description=Make this platform's distributions
Now would be a good opportunity to add comments to document what these
different targets are intended to do. I like to have a summary of the
'user oriented' targets at the top of the ant file. I think that 'one'
refers to build a distribution for just one platform, but someone else
might think it means building one permutation of the samples or
something like that.
http://gwt-code-reviews.appspot.com/37803/diff/1/2#newcode72
Line 72: antcall target=buildtools
Maybe a macrodef would help shrink the amount of code after getting rid
of the -do construct?
macrodef name=call-checkstyle
attribute name=target /
sequential
antcall targ...@{target}
param name=target value=checkstyle /
/antcall
/sequential
/macrodef
call-checkstyle target=buildtools /
call-checkstyle target=dev /
...
(I wish I could find something like a for loop)
http://gwt-code-reviews.appspot.com/37803/diff/1/2#newcode93
Line 93: antcall target=buildtools
same here - consider a macrodef.
http://gwt-code-reviews.appspot.com/37803
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
