Re: [gwt-contrib] Are there any docs describing internals of gwtc?
On Mon, Mar 14, 2011 at 4:41 PM, Grzegorz Kossakowski < grzegorz.kossakow...@gmail.com> wrote: > So, if my GWT app doesn't have any generators exact contents of > TypeOracle doesn't matter? I'm asking because I'm thinking of creating > some stub data structures for jribble units and just move on to fixing > some other issues and come back only once I want to support Generators > written in Scala or Generators referring Scala. > That *may* be true but you'd have to try it and find out. I know we use TypeOracle for a couple of other purposes in dev mode, but I don't *think* we use it for anything else in web mode.. > > 2) The GWT AST, which is essentially a source-level representation of the > > user's Java code, including method bodies, which is optimized and > translated > > into JavaScript. May be referred to as GWTC, the compiler, the web-mode > > compiler, etc. Mostly lives in com.google.gwt.dev.jjs (which stands for > > 'Java to JavaScript'). TypeMap is a small implementation detail that's > only > > useful for the JDT AST -> GWT AST translation. > > I see. It happens that TypeMap is quite interesting from my point of > view because I'm translating Jribble AST to GWT AST by following JDT > AST -> GWT AST translation (more or less). > Eek... to be honest, JDT AST -> GWT AST is some of the nastiest code around, mostly due to JDT. If you're going to follow a pattern, you might look at the new GwtAstBuilder, which is at least marginally cleaner than BuildTypeMap/GenerateJavaAST. > Is there one class that is dealing specifically with emulating object > orientation or it's scattered around many classes? I'm asking because > toString() call get's translated to toString_2() javascript call and > this method is not being defined anywhere in js source code. I'm > trying to pinpoint location where this stuff is handled to understand > why I get a call to non-existing place. > Hmm, I'm not completely sure what you're asking. GenerateJavaScriptAST is the place to look for how precisely the Java AST is transformed into JavaScript. > I've got another question. Do you have any tool for checking > correctness of GWT's ASTs or a tool that allows one to dump ASTs and > compare them? The scenario I have in mind is like this: > 1. I have minimal jribble example that exhibits some obscure problem > like with toString described above and I fail to find a bug but I > suspect that GWT ASTs I get out of Jribble are broken. > 2. I prepare corresponding Java code that imitate code from jribble. > This should work (or I found a bug in gwtc itself but this is highly > unlikely). Now I'd like to compare those two ASTs. Such comparison > should immediately reveal the source of my problems. > You can always call toSource() on any GWT AST node to get the source representation. GwtAstBuilderTest utilizes this to verify that the new GwtAstBuilder is mostly source-compatible with the old translator. We also have a bunch of unit tests derived from JJSTestBase that all generally verify that a certain input source + certain transformations turns into expected output. Other that that, we use JUnit / GWTTestCase to actually execute the compiled output and verify correctness JUnit style. Scott > > Do you have anything handy that would help me with this or do you have > some other idea how to solve my problem? > > -- > Best regards, > Grzegorz Kossakowski > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adding a new annotation SafeHtmlTemplates.SafeForCss to specify that a parameter is known to be ... (issue1384801)
http://gwt-code-reviews.appspot.com/1384801/diff/3003/user/test/com/google/gwt/safehtml/client/SafeHtmlTemplatesTest.java
File user/test/com/google/gwt/safehtml/client/SafeHtmlTemplatesTest.java
(right):
http://gwt-code-reviews.appspot.com/1384801/diff/3003/user/test/com/google/gwt/safehtml/client/SafeHtmlTemplatesTest.java#newcode80
user/test/com/google/gwt/safehtml/client/SafeHtmlTemplatesTest.java:80:
public void testTemplateWithCssAttribute() {
On 2011/03/14 22:01:18, jlabanca wrote:
On 2011/03/14 18:25:38, skybrian wrote:
> On 2011/03/14 17:03:18, jlabanca wrote:
> > The & character is valid because it can be used in a URL:
> > background:url(http://url?image=123112&size=1024)
>
> I'm wondering if this is more correct:
>
> background:url(http://url?image=123112&size=1024);
>
> Similarly, can we use other HTML entities like " within a
SafeCssProperties
> string?
I'm out of my element here. Is our assertion that users should not
use single
or double quotes? And will %26 escaping URLs might be better, it
certainly
isn't invalid to use & (I think). I'm not quite sure what I should be
testing
here since we aren't actually providing a SafeCssProperties escaping
feature
yet.
This might be excessive, but one way would be use
Element.getStyle().getProperty() to make sure that the property we get
out of the DOM is what we expect. The point isn't to test our
(nonexistent) code, but rather to test our assumptions about what
browsers actually do with weird characters in style attributes. (But I
don't know how to do it for a
[gwt-contrib] Re: Fix issue 5807 on server side. Previously reviewed at 1370803. (issue1384802)
http://gwt-code-reviews.appspot.com/1384802/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds a cache (PersistenUnitCache) to store CompilationUnits (issue1375802)
Here is a re-working of the cache based on the feedback. I've removed
caching inside CompilationStateBuilder, and made all cache lookups by
the ContentId.
Still TODO on this patch is a unit test, which I will work on tomorrow.
http://gwt-code-reviews.appspot.com/1375802/diff/1/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
File dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
(right):
http://gwt-code-reviews.appspot.com/1375802/diff/1/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java#newcode232
dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java:232:
logger.log(TreeLogger.TRACE, "Invalid units found: " +
invalidatedUnits.size());
On 2011/03/07 20:14:13, scottb wrote:
I would dig on this more. It's perfectly fine and expected that some
units will
have null dependencies. That should not force those units to
recompile...
*provided that the dependencies remain null*.
I think we need to go deeper...
When you say the dependencies remain null, remain from when until when?
Are you saying the Dependencies.validate() method needs revamping? or
that we add a special case for checking null dependencies?
http://gwt-code-reviews.appspot.com/1375802/diff/1/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java#newcode450
dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java:450:
On 2011/03/07 20:14:13, scottb wrote:
It does seem kind of strange, though, like the two need to be unified.
Maybe
PersistentUnitCache should be one of two UnitCache subclasses, or
maybe
internally it should either use the disk or not.
UnitCache is now an interface
MemoryUnitCache implements an in-memory cache
PersistentUnitCache extends MemoryUnitCache to back up and reload the
cache from disk.
http://gwt-code-reviews.appspot.com/1375802/diff/5002/dev/core/src/com/google/gwt/dev/Compiler.java
File dev/core/src/com/google/gwt/dev/Compiler.java (right):
http://gwt-code-reviews.appspot.com/1375802/diff/5002/dev/core/src/com/google/gwt/dev/Compiler.java#newcode204
dev/core/src/com/google/gwt/dev/Compiler.java:204: // that persists
between compilation runs and call PersistentCache.setCacheDir()
On 2011/03/08 16:29:31, jbrosenberg wrote:
s/call/calls to/
Done.
http://gwt-code-reviews.appspot.com/1375802/diff/5002/dev/core/src/com/google/gwt/dev/DevMode.java
File dev/core/src/com/google/gwt/dev/DevMode.java (right):
http://gwt-code-reviews.appspot.com/1375802/diff/5002/dev/core/src/com/google/gwt/dev/DevMode.java#newcode417
dev/core/src/com/google/gwt/dev/DevMode.java:417: File
persistentCacheDir = new File(options.getWarDir(),
"/WEB-INF/gwt-unitCache");
On 2011/03/08 02:31:25, tobyr wrote:
Make "gwt-unitCache" a constant somewhere (and replace all uses)?
Done.
http://gwt-code-reviews.appspot.com/1375802/diff/5002/dev/core/src/com/google/gwt/dev/DevModeBase.java
File dev/core/src/com/google/gwt/dev/DevModeBase.java (right):
http://gwt-code-reviews.appspot.com/1375802/diff/5002/dev/core/src/com/google/gwt/dev/DevModeBase.java#newcode889
dev/core/src/com/google/gwt/dev/DevModeBase.java:889:
PersistentUnitCache.init(getTopLogger().branch(TreeLogger.INFO,
On 2011/03/08 02:31:25, tobyr wrote:
Why not just branch the TreeLogger inside of init?
Done.
http://gwt-code-reviews.appspot.com/1375802/diff/5002/dev/core/src/com/google/gwt/dev/DevModeBase.java#newcode1061
dev/core/src/com/google/gwt/dev/DevModeBase.java:1061: }
On 2011/03/08 16:29:31, jbrosenberg wrote:
white space
Done.
http://gwt-code-reviews.appspot.com/1375802/diff/5002/dev/core/src/com/google/gwt/dev/GWTCompiler.java
File dev/core/src/com/google/gwt/dev/GWTCompiler.java (right):
http://gwt-code-reviews.appspot.com/1375802/diff/5002/dev/core/src/com/google/gwt/dev/GWTCompiler.java#newcode180
dev/core/src/com/google/gwt/dev/GWTCompiler.java:180: } else {
On 2011/03/08 02:31:25, tobyr wrote:
This logic was a bit confusing to follow. Mind setting
persistentCacheDir to
null in the if block instead of initializing it to null? A comment
might help
too.
Done.
http://gwt-code-reviews.appspot.com/1375802/diff/5002/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
File dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
(left):
http://gwt-code-reviews.appspot.com/1375802/diff/5002/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java#oldcode422
dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java:422:
CompilationUnit existingUnit = unitCache.get(contentId);
On 2011/03/08 02:31:25, tobyr wrote:
Put the check of the unitCache and PUC together (for example, in a
method) so
you don't duplicate the the cachedUnits.put/compileMoreLater logic.
Or...
replace unitCache with PUC altogether.
Done.
http://gwt-code-reviews.appspot.com/1375802/diff/5002/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
File dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
(right):
http://
[gwt-contrib] Re: Adds a cache (PersistenUnitCache) to store CompilationUnits (issue1375802)
http://gwt-code-reviews.appspot.com/1375802/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adding a new annotation SafeHtmlTemplates.SafeForCss to specify that a parameter is known to be ... (issue1384801)
http://gwt-code-reviews.appspot.com/1384801/diff/6006/user/src/com/google/gwt/safecss/shared/SafeCssProperties.java
File user/src/com/google/gwt/safecss/shared/SafeCssProperties.java
(right):
http://gwt-code-reviews.appspot.com/1384801/diff/6006/user/src/com/google/gwt/safecss/shared/SafeCssProperties.java#newcode46
user/src/com/google/gwt/safecss/shared/SafeCssProperties.java:46: * By
convention, {@link SafeCssProperties} should only contain single quotes
Since SafeHtmlTemplates has been changed to HTML-escape the value of
style attributes, perhaps it might avoid some confusion to remove the
suggestion about the quotes.
It wouldn't hurt to instead remind users that SafeCssProperties strings
may contain literal single or double quotes, and as such the entire CSS
must be HTML escaped when used in a style attribute.
One thing that is important to require is that SafeCssProperties may
never contain literal angle brackets. Otherwise, it could be unsafe to
place a SafeCssProperties into a tag (where it can't be HTML
escaped), e.g. if the SafeCssProperties such as
font: 'foo evil'
is used in a style sheet in a
[gwt-contrib] Re: Adding a new annotation SafeHtmlTemplates.SafeForCss to specify that a parameter is known to be ... (issue1384801)
http://gwt-code-reviews.appspot.com/1384801/diff/3003/user/test/com/google/gwt/safehtml/client/SafeHtmlTemplatesTest.java
File user/test/com/google/gwt/safehtml/client/SafeHtmlTemplatesTest.java
(right):
http://gwt-code-reviews.appspot.com/1384801/diff/3003/user/test/com/google/gwt/safehtml/client/SafeHtmlTemplatesTest.java#newcode80
user/test/com/google/gwt/safehtml/client/SafeHtmlTemplatesTest.java:80:
public void testTemplateWithCssAttribute() {
On 2011/03/14 18:25:38, skybrian wrote:
On 2011/03/14 17:03:18, jlabanca wrote:
> The & character is valid because it can be used in a URL:
> background:url(http://url?image=123112&size=1024)
I'm wondering if this is more correct:
background:url(http://url?image=123112&size=1024);
Similarly, can we use other HTML entities like " within a
SafeCssProperties
string?
I'm out of my element here. Is our assertion that users should not use
single or double quotes? And will %26 escaping URLs might be better, it
certainly isn't invalid to use & (I think). I'm not quite sure what I
should be testing here since we aren't actually providing a
SafeCssProperties escaping feature yet.
http://gwt-code-reviews.appspot.com/1384801/diff/3008/user/src/com/google/gwt/cell/client/IconCellDecorator.java
File user/src/com/google/gwt/cell/client/IconCellDecorator.java (right):
http://gwt-code-reviews.appspot.com/1384801/diff/3008/user/src/com/google/gwt/cell/client/IconCellDecorator.java#newcode196
user/src/com/google/gwt/cell/client/IconCellDecorator.java:196: String
cssString = direction + ":0px;";
On 2011/03/14 18:25:38, skybrian wrote:
To remove a bit of duplication, maybe make this variable have type
SafeCssProperties?
Done.
http://gwt-code-reviews.appspot.com/1384801/diff/3008/user/src/com/google/gwt/cell/client/IconCellDecorator.java#newcode203
user/src/com/google/gwt/cell/client/IconCellDecorator.java:203:
cssString += "margin-top:-" + halfHeight + "px;";
On 2011/03/14 18:25:38, skybrian wrote:
We should have some way of concatenating two variables of type
SafeCssProperties.
Done.
I added SafeCssPropertiesBuilder and used it here and in other places.
http://gwt-code-reviews.appspot.com/1384801/diff/3008/user/src/com/google/gwt/safecss/shared/SafeCssProperties.java
File user/src/com/google/gwt/safecss/shared/SafeCssProperties.java
(right):
http://gwt-code-reviews.appspot.com/1384801/diff/3008/user/src/com/google/gwt/safecss/shared/SafeCssProperties.java#newcode107
user/src/com/google/gwt/safecss/shared/SafeCssProperties.java:107: * -
Some implementations of SafeCssProperties would in principle be able to
On 2011/03/14 21:07:51, xtof wrote:
This sentence got formatted wrong?
Done.
http://gwt-code-reviews.appspot.com/1384801/diff/3008/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java
File
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java
(right):
http://gwt-code-reviews.appspot.com/1384801/diff/3008/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java#newcode185
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java:185:
// SafeCss is safe in a CSS context, so we can use the string without
On 2011/03/14 21:07:51, xtof wrote:
I think we should HTML escape the values of style= attributes; this
will prevent
the attribute-escape bug that's mentioned in the SafeHtmlProperties
documentation.
Unless I'm totally confused, the browser will first HTML-unescape the
style
attribute and then pass it to the CSS parser, i.e. HTML escaping
quotes and
single quotes (and <> markup) will not change behavior, but will make
sure the
HTML attribute is correctly parsed in its entirety.
Done.
I didn't realize the browser would unescape it. Escaping seems to work
fine.
http://gwt-code-reviews.appspot.com/1384801/diff/3008/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java#newcode264
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java:264:
*
On 2011/03/14 21:07:51, xtof wrote:
Trailing blank, here and below...
Done.
http://gwt-code-reviews.appspot.com/1384801/diff/3008/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java#newcode284
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java:284:
* Verify that the parameter type if used in the correct context. Safe
On 2011/03/14 21:07:51, xtof wrote:
"is used"?
Done.
http://gwt-code-reviews.appspot.com/1384801/diff/3008/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java#newcode328
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java:328:
// Warn against using unsafe parameters in a CSS attribute context.
On 2011/03/14 21:07:51, xtof wrote:
It's worth noting that even if a SafeCssProperties is used, the
template isn't
100% guaranteed to be safe, because we don't know if the template
variable
appears in a "composable" CSS context.
[gwt-contrib] Re: Adding a new annotation SafeHtmlTemplates.SafeForCss to specify that a parameter is known to be ... (issue1384801)
http://gwt-code-reviews.appspot.com/1384801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adding a new annotation SafeHtmlTemplates.SafeForCss to specify that a parameter is known to be ... (issue1384801)
http://gwt-code-reviews.appspot.com/1384801/diff/3008/user/src/com/google/gwt/safecss/shared/SafeCssProperties.java
File user/src/com/google/gwt/safecss/shared/SafeCssProperties.java
(right):
http://gwt-code-reviews.appspot.com/1384801/diff/3008/user/src/com/google/gwt/safecss/shared/SafeCssProperties.java#newcode107
user/src/com/google/gwt/safecss/shared/SafeCssProperties.java:107: * -
Some implementations of SafeCssProperties would in principle be able to
This sentence got formatted wrong?
http://gwt-code-reviews.appspot.com/1384801/diff/3008/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java
File
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java
(right):
http://gwt-code-reviews.appspot.com/1384801/diff/3008/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java#newcode185
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java:185:
// SafeCss is safe in a CSS context, so we can use the string without
I think we should HTML escape the values of style= attributes; this will
prevent the attribute-escape bug that's mentioned in the
SafeHtmlProperties documentation.
Unless I'm totally confused, the browser will first HTML-unescape the
style attribute and then pass it to the CSS parser, i.e. HTML escaping
quotes and single quotes (and <> markup) will not change behavior, but
will make sure the HTML attribute is correctly parsed in its entirety.
http://gwt-code-reviews.appspot.com/1384801/diff/3008/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java#newcode264
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java:264:
*
Trailing blank, here and below...
http://gwt-code-reviews.appspot.com/1384801/diff/3008/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java#newcode284
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java:284:
* Verify that the parameter type if used in the correct context. Safe
"is used"?
http://gwt-code-reviews.appspot.com/1384801/diff/3008/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java#newcode328
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java:328:
// Warn against using unsafe parameters in a CSS attribute context.
It's worth noting that even if a SafeCssProperties is used, the template
isn't 100% guaranteed to be safe, because we don't know if the template
variable appears in a "composable" CSS context. For example,
@Template(""
SafeHtml div(SafeCssProperties safeCss);
will not result in a compiler warning, and _could_ potentially mask an
exploitable bug -- since CSS quotes in the parameter are "inside out", a
CSS property escape could happen, e.g if the value of CSS is something
like,
http://foo'; background:url(javascript:evil()); font'
Now, it's very unlikely that the code that constructs safeCss will allow
this, but the potential exists.
There are a few ways to clean this up:
- Augment the stream parser to dive into CSS and determine CSS context
(I'm not sure how hard this is; presumably not easy, otherwise they'd
done it)
- Only allow template variables in CSS_ATTRIBUTE context to appear at
the very beginning of the style attribute (without warning). This by
definition is a composable CSS context we can be sure of. This smells a
bit awkward, but might be an overall reasonable solution.
http://gwt-code-reviews.appspot.com/1384801/diff/3008/user/src/com/google/gwt/safehtml/shared/SafeHtml.java
File user/src/com/google/gwt/safehtml/shared/SafeHtml.java (right):
http://gwt-code-reviews.appspot.com/1384801/diff/3008/user/src/com/google/gwt/safehtml/shared/SafeHtml.java#newcode63
user/src/com/google/gwt/safehtml/shared/SafeHtml.java:63: * Notes
regarding serialization: - It may be reasonable to allow
This got formatted funny as well at some point.
http://gwt-code-reviews.appspot.com/1384801/diff/3008/user/src/com/google/gwt/user/cellview/client/CellTree.java
File user/src/com/google/gwt/user/cellview/client/CellTree.java (right):
http://gwt-code-reviews.appspot.com/1384801/diff/3008/user/src/com/google/gwt/user/cellview/client/CellTree.java#newcode954
user/src/com/google/gwt/user/cellview/client/CellTree.java:954:
cssBuilder.append("width: " + res.getWidth() + "px;");
I wonder if code like this might be a bit more readable if
SafeCssPropertiesBuilder was used. Perhaps to avoid excessive verbosity,
we could have
safeCssBuilder.appendFromTrustedString("width:" + .. + "px;")
as a shorthand for
safeCssBuilder.append(SafeCssUtils.fromTrustedString(...));
The advantage is that the code can then be written such that each append
adds a single CSS property, and to code review for correctness I have to
only eyeball each .append individually rather than having to consider
the whole builder. Perhaps not a big deal in practice though.
http://gwt-code-reviews.appspot.com/1384801/
--
http://groups.google.com/gr
Re: [gwt-contrib] Are there any docs describing internals of gwtc?
Hi Scott! Thanks for speedy and seriously awesome response! :-) Some questions inline. 2011/3/14 Scott Blum : > On Mon, Mar 14, 2011 at 3:03 PM, Grzegorz Kossakowski > wrote: >> >> 1. What's the difference between TypeMap and TypeOracle and why they >> seem to have overlapping functionality? > > In GWT, there are two major pieces of infrastructure that deal with > representing Java language input. > 1) A reflection of the user's type model for the purposes of running > generators. Approximately equivalent to the kind of information you can get > via Java reflection. Models types, fields, and methods, but not method > bodies. The entire machinery around this can be loosely referred to as > "TypeOracle", which is the primary interface for accessing this type model. > Generators use this type model to generate code. This infrastructure is > used in both dev mode and web mode, as Generators run in both contexts. So, if my GWT app doesn't have any generators exact contents of TypeOracle doesn't matter? I'm asking because I'm thinking of creating some stub data structures for jribble units and just move on to fixing some other issues and come back only once I want to support Generators written in Scala or Generators referring Scala. I'm asking because I don't want to create more problems than I solve at the time. > 2) The GWT AST, which is essentially a source-level representation of the > user's Java code, including method bodies, which is optimized and translated > into JavaScript. May be referred to as GWTC, the compiler, the web-mode > compiler, etc. Mostly lives in com.google.gwt.dev.jjs (which stands for > 'Java to JavaScript'). TypeMap is a small implementation detail that's only > useful for the JDT AST -> GWT AST translation. I see. It happens that TypeMap is quite interesting from my point of view because I'm translating Jribble AST to GWT AST by following JDT AST -> GWT AST translation (more or less). It's good to know that it's not being used anywhere else, though. >> >> 2. Why TypeOracleMediator operates on bytecode whereas gwtc is >> supposed to work with java source, or jdt's asts? > > Historically, we've run JDT twice. Once to build TypeOracle, and once to > build the GWT AST. Efforts have been made to transition TypeOracle to be > built from bytecode, which would eliminate one of these JDT runs in favor of > using external (presumably IDE-built) class files. I see. Thanks for clarification. >> >> 3. How emulation of object orientation is implemented in gwt? > > Using JavaScript prototype chains, and mangling method signatures such that > any given override of a method has a globally unique name in the compiled > output. Try compiling with -style PRETTY or DETAILED and inspect the > output. Is there one class that is dealing specifically with emulating object orientation or it's scattered around many classes? I'm asking because toString() call get's translated to toString_2() javascript call and this method is not being defined anywhere in js source code. I'm trying to pinpoint location where this stuff is handled to understand why I get a call to non-existing place. >> >> 4. What are main phases in gwtc's execution. What are dependencies? > > JavaToJavaScriptCompiler .precompile() is what I would consider the main > high-level algorithm. But here's a text overview. > 1) Build TypeOracle from the initial set of available source units > (CompilationStateBuilder). > 2) Begin running the web mode compile (WebModeCompilerFrontEnd). > 3) As the web mode compile runs, we encounter GWT.create() calls > (FindDeferredBindingSitesVisitor). > 4) Use the rebind infrastructure to resolve rebind requests. This may > entail running Generators, which generate new code. > 5) Newly-generated types get added to CompilationState + TypeOracle in a bit > of a loop-back process. > 6) Newly-generated types may themselves contain GWT.create() calls, looping > back through step 4. > 7) Eventually, all our code is generated. Get rid of TypeOracle, we don't > need it anymore. > 8) GenerateJavaAST turns the JDT AST into the GWT AST. > 9) Normalize certain difficult constructs. > 10) Optimize. > 11) Post-optimization normalizes to turn high-level Java constructs into > JS-specific implementation details (but still Java AST). > 12) GenerateJavaScriptAST > 13) Optimize JS AST > 14) Produce JS source text. That looks great. Thanks! >> >> Do you guys have anything (slides, blog posts, discussions, etc.) that >> would help me to better understand those matters? > > Here's wiki entries for TypeOracle / CompilationState. The first is > outdated by useful to understand the second one. > http://code.google.com/p/google-web-toolkit/wiki/CompilationUnit_1_5 > http://code.google.com/p/google-web-toolkit/wiki/CompilationUnit Thanks. This helps to understand another bit of GWT's architecture. I've got another question. Do you have any tool for checking correctness of GWT's ASTs or a tool that allows one to
[gwt-contrib] Re: Reintroduces JsInliner patch with minor tweaks (previously submitted by cromwellian at rev 9362) (issue1386801)
LGTM http://gwt-code-reviews.appspot.com/1386801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Reintroduces JsInliner patch with minor tweaks (previously submitted by cromwellian at rev 9362) (issue1386801)
http://gwt-code-reviews.appspot.com/1386801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Are there any docs describing internals of gwtc?
On Mon, Mar 14, 2011 at 3:03 PM, Grzegorz Kossakowski < grzegorz.kossakow...@gmail.com> wrote: > 1. What's the difference between TypeMap and TypeOracle and why they > seem to have overlapping functionality? > In GWT, there are two major pieces of infrastructure that deal with representing Java language input. 1) A reflection of the user's type model for the purposes of running generators. Approximately equivalent to the kind of information you can get via Java reflection. Models types, fields, and methods, but not method bodies. The entire machinery around this can be loosely referred to as "TypeOracle", which is the primary interface for accessing this type model. Generators use this type model to generate code. This infrastructure is used in both dev mode and web mode, as Generators run in both contexts. 2) The GWT AST, which is essentially a source-level representation of the user's Java code, including method bodies, which is optimized and translated into JavaScript. May be referred to as GWTC, the compiler, the web-mode compiler, etc. Mostly lives in com.google.gwt.dev.jjs (which stands for 'Java to JavaScript'). TypeMap is a small implementation detail that's only useful for the JDT AST -> GWT AST translation. > 2. Why TypeOracleMediator operates on bytecode whereas gwtc is > supposed to work with java source, or jdt's asts? > Historically, we've run JDT twice. Once to build TypeOracle, and once to build the GWT AST. Efforts have been made to transition TypeOracle to be built from bytecode, which would eliminate one of these JDT runs in favor of using external (presumably IDE-built) class files. > 3. How emulation of object orientation is implemented in gwt? > Using JavaScript prototype chains, and mangling method signatures such that any given override of a method has a globally unique name in the compiled output. Try compiling with -style PRETTY or DETAILED and inspect the output. > 4. What are main phases in gwtc's execution. What are dependencies? > JavaToJavaScriptCompiler .precompile() is what I would consider the main high-level algorithm. But here's a text overview. 1) Build TypeOracle from the initial set of available source units (CompilationStateBuilder). 2) Begin running the web mode compile (WebModeCompilerFrontEnd). 3) As the web mode compile runs, we encounter GWT.create() calls (FindDeferredBindingSitesVisitor). 4) Use the rebind infrastructure to resolve rebind requests. This may entail running Generators, which generate new code. 5) Newly-generated types get added to CompilationState + TypeOracle in a bit of a loop-back process. 6) Newly-generated types may themselves contain GWT.create() calls, looping back through step 4. 7) Eventually, all our code is generated. Get rid of TypeOracle, we don't need it anymore. 8) GenerateJavaAST turns the JDT AST into the GWT AST. 9) Normalize certain difficult constructs. 10) Optimize. 11) Post-optimization normalizes to turn high-level Java constructs into JS-specific implementation details (but still Java AST). 12) GenerateJavaScriptAST 13) Optimize JS AST 14) Produce JS source text. > I'd like to know where TypeMap is being built and why it's done twice. > It should only get built once. > Also, TypeOracle seems to be highly mutable data structure. When it > gets updated and why? > >From the outside, it should be seen as "append only". Existing state should be effectively immutable. It gets updated because Generators produce new types as the compile proceeds. > Do you guys have anything (slides, blog posts, discussions, etc.) that > would help me to better understand those matters? > Here's wiki entries for TypeOracle / CompilationState. The first is outdated by useful to understand the second one. http://code.google.com/p/google-web-toolkit/wiki/CompilationUnit_1_5 http://code.google.com/p/google-web-toolkit/wiki/CompilationUnit -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Are there any docs describing internals of gwtc?
Hi, More and more often I find myself lacking the big picture of gwtc's internals. I'd like to find answers (or hints where to look for answers) for questions like: 1. What's the difference between TypeMap and TypeOracle and why they seem to have overlapping functionality? 2. Why TypeOracleMediator operates on bytecode whereas gwtc is supposed to work with java source, or jdt's asts? 3. How emulation of object orientation is implemented in gwt? 4. What are main phases in gwtc's execution. What are dependencies? I'd like to know where TypeMpa is being built and why it's done twice. Also, TypeOracle seems to be highly mutable data structure. When it gets updated and why? Do you guys have anything (slides, blog posts, discussions, etc.) that would help me to better understand those matters? -- Grzegorz Kossakowski -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixing touch scrolling support for devices that support it natively. Our current touch support o... (issue1369809)
On 2011/03/14 18:29:43, jlabanca wrote:
http://gwt-code-reviews.appspot.com/1369809/diff/4001/user/src/com/google/gwt/event/dom/DomEvent.gwt.xml
File user/src/com/google/gwt/event/dom/DomEvent.gwt.xml (right):
http://gwt-code-reviews.appspot.com/1369809/diff/4001/user/src/com/google/gwt/event/dom/DomEvent.gwt.xml#newcode3
user/src/com/google/gwt/event/dom/DomEvent.gwt.xml:3:
On 2011/03/14 18:09:14, pdr wrote:
> Can you alphabetize these?
Done.
http://gwt-code-reviews.appspot.com/1369809/diff/4001/user/src/com/google/gwt/event/dom/client/TouchEvent.java
File user/src/com/google/gwt/event/dom/client/TouchEvent.java (right):
http://gwt-code-reviews.appspot.com/1369809/diff/4001/user/src/com/google/gwt/event/dom/client/TouchEvent.java#newcode35
user/src/com/google/gwt/event/dom/client/TouchEvent.java:35: private
static
class TouchSupportDetector {
This would return true for a tablet Android device. TouchScroller has
the
second check for Android, as you described.
http://gwt-code-reviews.appspot.com/1369809/diff/4001/user/src/com/google/gwt/user/tools/templates/eclipse/_moduleShortName_.launch
File
user/src/com/google/gwt/user/tools/templates/eclipse/_moduleShortName_.launch
(left):
http://gwt-code-reviews.appspot.com/1369809/diff/4001/user/src/com/google/gwt/user/tools/templates/eclipse/_moduleShortName_.launch#oldcode1
user/src/com/google/gwt/user/tools/templates/eclipse/_moduleShortName_.launch:1:
On 2011/03/14 18:09:14, pdr wrote:
> Was deleting this a mistake?
Yes, reverting
http://gwt-code-reviews.appspot.com/1369809/diff/4001/user/test/com/google/gwt/touch/client/TouchScrollTest.java
File user/test/com/google/gwt/touch/client/TouchScrollTest.java
(right):
http://gwt-code-reviews.appspot.com/1369809/diff/4001/user/test/com/google/gwt/touch/client/TouchScrollTest.java#newcode513
user/test/com/google/gwt/touch/client/TouchScrollTest.java:513: }
isTouchSupported() was a helper method to determine if TouchScroller
should be
supported in this test. However, now that the detection is more
complicated,
copying TouchScroller.isSupport() seems redundant. I agree in
principle that it
would be nice to verify that TouchScroller.isSupported() returns the
correct
value on the correct device, but if we just copy the implementation of
TouchScroller.isSupported(), then it isn't really testing anything.
LGTM
http://gwt-code-reviews.appspot.com/1369809/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adding a new annotation SafeHtmlTemplates.SafeForCss to specify that a parameter is known to be ... (issue1384801)
Seems basically fine. I'm just wondering if we're missing anything.
http://gwt-code-reviews.appspot.com/1384801/diff/3003/user/test/com/google/gwt/safehtml/client/SafeHtmlTemplatesTest.java
File user/test/com/google/gwt/safehtml/client/SafeHtmlTemplatesTest.java
(right):
http://gwt-code-reviews.appspot.com/1384801/diff/3003/user/test/com/google/gwt/safehtml/client/SafeHtmlTemplatesTest.java#newcode80
user/test/com/google/gwt/safehtml/client/SafeHtmlTemplatesTest.java:80:
public void testTemplateWithCssAttribute() {
On 2011/03/14 17:03:18, jlabanca wrote:
The & character is valid because it can be used in a URL:
background:url(http://url?image=123112&size=1024)
I'm wondering if this is more correct:
background:url(http://url?image=123112&size=1024);
Similarly, can we use other HTML entities like " within a
SafeCssProperties string?
http://gwt-code-reviews.appspot.com/1384801/diff/3008/user/src/com/google/gwt/cell/client/IconCellDecorator.java
File user/src/com/google/gwt/cell/client/IconCellDecorator.java (right):
http://gwt-code-reviews.appspot.com/1384801/diff/3008/user/src/com/google/gwt/cell/client/IconCellDecorator.java#newcode196
user/src/com/google/gwt/cell/client/IconCellDecorator.java:196: String
cssString = direction + ":0px;";
To remove a bit of duplication, maybe make this variable have type
SafeCssProperties?
http://gwt-code-reviews.appspot.com/1384801/diff/3008/user/src/com/google/gwt/cell/client/IconCellDecorator.java#newcode203
user/src/com/google/gwt/cell/client/IconCellDecorator.java:203:
cssString += "margin-top:-" + halfHeight + "px;";
We should have some way of concatenating two variables of type
SafeCssProperties.
http://gwt-code-reviews.appspot.com/1384801/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixing touch scrolling support for devices that support it natively. Our current touch support o... (issue1369809)
http://gwt-code-reviews.appspot.com/1369809/diff/4001/user/src/com/google/gwt/event/dom/DomEvent.gwt.xml
File user/src/com/google/gwt/event/dom/DomEvent.gwt.xml (right):
http://gwt-code-reviews.appspot.com/1369809/diff/4001/user/src/com/google/gwt/event/dom/DomEvent.gwt.xml#newcode3
user/src/com/google/gwt/event/dom/DomEvent.gwt.xml:3:
On 2011/03/14 18:09:14, pdr wrote:
Can you alphabetize these?
Done.
http://gwt-code-reviews.appspot.com/1369809/diff/4001/user/src/com/google/gwt/event/dom/client/TouchEvent.java
File user/src/com/google/gwt/event/dom/client/TouchEvent.java (right):
http://gwt-code-reviews.appspot.com/1369809/diff/4001/user/src/com/google/gwt/event/dom/client/TouchEvent.java#newcode35
user/src/com/google/gwt/event/dom/client/TouchEvent.java:35: private
static class TouchSupportDetector {
This would return true for a tablet Android device. TouchScroller has
the second check for Android, as you described.
http://gwt-code-reviews.appspot.com/1369809/diff/4001/user/src/com/google/gwt/user/tools/templates/eclipse/_moduleShortName_.launch
File
user/src/com/google/gwt/user/tools/templates/eclipse/_moduleShortName_.launch
(left):
http://gwt-code-reviews.appspot.com/1369809/diff/4001/user/src/com/google/gwt/user/tools/templates/eclipse/_moduleShortName_.launch#oldcode1
user/src/com/google/gwt/user/tools/templates/eclipse/_moduleShortName_.launch:1:
On 2011/03/14 18:09:14, pdr wrote:
Was deleting this a mistake?
Yes, reverting
http://gwt-code-reviews.appspot.com/1369809/diff/4001/user/test/com/google/gwt/touch/client/TouchScrollTest.java
File user/test/com/google/gwt/touch/client/TouchScrollTest.java (right):
http://gwt-code-reviews.appspot.com/1369809/diff/4001/user/test/com/google/gwt/touch/client/TouchScrollTest.java#newcode513
user/test/com/google/gwt/touch/client/TouchScrollTest.java:513: }
isTouchSupported() was a helper method to determine if TouchScroller
should be supported in this test. However, now that the detection is
more complicated, copying TouchScroller.isSupport() seems redundant. I
agree in principle that it would be nice to verify that
TouchScroller.isSupported() returns the correct value on the correct
device, but if we just copy the implementation of
TouchScroller.isSupported(), then it isn't really testing anything.
http://gwt-code-reviews.appspot.com/1369809/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixing touch scrolling support for devices that support it natively. Our current touch support o... (issue1369809)
I had reviewed this after all, but forgot to click send. D'oh! Sorry for
the delay.
http://gwt-code-reviews.appspot.com/1369809/diff/4001/user/src/com/google/gwt/event/dom/DomEvent.gwt.xml
File user/src/com/google/gwt/event/dom/DomEvent.gwt.xml (right):
http://gwt-code-reviews.appspot.com/1369809/diff/4001/user/src/com/google/gwt/event/dom/DomEvent.gwt.xml#newcode3
user/src/com/google/gwt/event/dom/DomEvent.gwt.xml:3:
Can you alphabetize these?
http://gwt-code-reviews.appspot.com/1369809/diff/4001/user/src/com/google/gwt/event/dom/client/TouchEvent.java
File user/src/com/google/gwt/event/dom/client/TouchEvent.java (right):
http://gwt-code-reviews.appspot.com/1369809/diff/4001/user/src/com/google/gwt/event/dom/client/TouchEvent.java#newcode35
user/src/com/google/gwt/event/dom/client/TouchEvent.java:35: private
static class TouchSupportDetector {
This name is a bit confusing since it will return false for a tablet
Android device, which does fire touch events. If that fact is
javadoc'ed, I think things would be ok. Alternatively, have
TouchSupportDetector and a separate check for the touch-based scrolling.
http://gwt-code-reviews.appspot.com/1369809/diff/4001/user/src/com/google/gwt/user/tools/templates/eclipse/_moduleShortName_.launch
File
user/src/com/google/gwt/user/tools/templates/eclipse/_moduleShortName_.launch
(left):
http://gwt-code-reviews.appspot.com/1369809/diff/4001/user/src/com/google/gwt/user/tools/templates/eclipse/_moduleShortName_.launch#oldcode1
user/src/com/google/gwt/user/tools/templates/eclipse/_moduleShortName_.launch:1:
Was deleting this a mistake?
http://gwt-code-reviews.appspot.com/1369809/diff/4001/user/test/com/google/gwt/touch/client/TouchScrollTest.java
File user/test/com/google/gwt/touch/client/TouchScrollTest.java (right):
http://gwt-code-reviews.appspot.com/1369809/diff/4001/user/test/com/google/gwt/touch/client/TouchScrollTest.java#newcode513
user/test/com/google/gwt/touch/client/TouchScrollTest.java:513: }
You removed a test called isTouchSupported() that I was fond of. I'd
recommend adding a test to double-check that your support check works.
Something like if(browser == android && tablet && elem.ontouchstart ==
"function") { assertTrue(TouchScroller.isSupported()); }
http://gwt-code-reviews.appspot.com/1369809/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixing touch scrolling support for devices that support it natively. Our current touch support o... (issue1369809)
http://gwt-code-reviews.appspot.com/1369809/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix bug on IE where onload events don't fire for IFrames. (issue1294801)
LGTM http://gwt-code-reviews.appspot.com/1294801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix bug on IE where onload events don't fire for IFrames. (issue1294801)
http://gwt-code-reviews.appspot.com/1294801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix bug on IE where onload events don't fire for IFrames. (issue1294801)
http://gwt-code-reviews.appspot.com/1294801/diff/17001/user/src/com/google/gwt/user/client/ui/Frame.java File user/src/com/google/gwt/user/client/ui/Frame.java (right): http://gwt-code-reviews.appspot.com/1294801/diff/17001/user/src/com/google/gwt/user/client/ui/Frame.java#newcode78 user/src/com/google/gwt/user/client/ui/Frame.java:78: sinkEvents(Event.ONLOAD); On 2011/03/14 15:10:05, jlabanca wrote: Don't sink in the constructor unless its used by the Widget itself. addDomHandler will lazily sink the load event automatically. This is important because sinking events is expensive. Done. http://gwt-code-reviews.appspot.com/1294801/diff/17001/user/src/com/google/gwt/user/client/ui/Frame.java#newcode110 user/src/com/google/gwt/user/client/ui/Frame.java:110: return addHandler(handler, LoadEvent.getType()); On 2011/03/14 15:10:05, jlabanca wrote: Use addDomHandler() for events that wrap native events. It will sink ONLOAD the first time it is called. Done. http://gwt-code-reviews.appspot.com/1294801/diff/17001/user/test/com/google/gwt/dom/public-test/iframetest.html File user/test/com/google/gwt/dom/public-test/iframetest.html (right): http://gwt-code-reviews.appspot.com/1294801/diff/17001/user/test/com/google/gwt/dom/public-test/iframetest.html#newcode3 user/test/com/google/gwt/dom/public-test/iframetest.html:3: On 2011/03/14 15:10:05, jlabanca wrote: This doesn't look right svn diff!! >:( fixed :) http://gwt-code-reviews.appspot.com/1294801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adding a new annotation SafeHtmlTemplates.SafeForCss to specify that a parameter is known to be ... (issue1384801)
http://gwt-code-reviews.appspot.com/1384801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix bug on IE where onload events don't fire for IFrames. (issue1294801)
iframetest.html still looks weird. I'll assume it looks correct on your system. http://gwt-code-reviews.appspot.com/1294801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix bug on IE where onload events don't fire for IFrames. (issue1294801)
http://gwt-code-reviews.appspot.com/1294801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix bug on IE where onload events don't fire for IFrames. (issue1294801)
LGTM http://gwt-code-reviews.appspot.com/1294801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adding a new annotation SafeHtmlTemplates.SafeForCss to specify that a parameter is known to be ... (issue1384801)
http://gwt-code-reviews.appspot.com/1384801/diff/3003/user/src/com/google/gwt/safecss/shared/SafeCssProperties.java
File user/src/com/google/gwt/safecss/shared/SafeCssProperties.java
(right):
http://gwt-code-reviews.appspot.com/1384801/diff/3003/user/src/com/google/gwt/safecss/shared/SafeCssProperties.java#newcode37
user/src/com/google/gwt/safecss/shared/SafeCssProperties.java:37: * All
implementing classes must maintain the class invariant (by design and
On 2011/03/11 23:02:45, skybrian wrote:
This description of the class invariant is rather abstract. I think
users would
find it helpful if we gave them some examples of valid and invalid
strings.
As I understand it, the empty string, "width: 1em;" and "width: 1em;
height:
1em;" are safe, but just "1em" is not. It's also an error to leave off
the
trailing semicolon.
Done.
http://gwt-code-reviews.appspot.com/1384801/diff/3003/user/src/com/google/gwt/safecss/shared/SafeCssPropertiesString.java
File user/src/com/google/gwt/safecss/shared/SafeCssPropertiesString.java
(right):
http://gwt-code-reviews.appspot.com/1384801/diff/3003/user/src/com/google/gwt/safecss/shared/SafeCssPropertiesString.java#newcode40
user/src/com/google/gwt/safecss/shared/SafeCssPropertiesString.java:40:
SafeCssPropertiesString(String css) {
On 2011/03/11 23:02:45, skybrian wrote:
perhaps have an assert that the string is either empty or ends with a
semicolon,
just to catch blatantly wrong usage?
Done.
http://gwt-code-reviews.appspot.com/1384801/diff/3003/user/src/com/google/gwt/safecss/shared/SafeCssPropertiesUtils.java
File user/src/com/google/gwt/safecss/shared/SafeCssPropertiesUtils.java
(right):
http://gwt-code-reviews.appspot.com/1384801/diff/3003/user/src/com/google/gwt/safecss/shared/SafeCssPropertiesUtils.java#newcode31
user/src/com/google/gwt/safecss/shared/SafeCssPropertiesUtils.java:31: *
code should be carefully reviewed to ensure the argument meets the
On 2011/03/11 23:02:45, skybrian wrote:
As a convenience to the user, maybe put some concrete examples here
too. (I
suspect this JavaDoc would be the first documentation seen by someone
reading
user code in an IDE, and concrete examples will make it blindingly
obvious if
someone's doing it wrong.)
Done.
http://gwt-code-reviews.appspot.com/1384801/diff/3003/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java
File
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java
(right):
http://gwt-code-reviews.appspot.com/1384801/diff/3003/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java#newcode146
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java:146:
* If the parameter is not of type {@link String}, it is first
converted
On 2011/03/11 23:02:45, skybrian wrote:
need to update javadoc
Done.
http://gwt-code-reviews.appspot.com/1384801/diff/3003/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java#newcode185
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java:185:
doEscape = false;
I think escaping at this point would defeat the purpose of using
SafeCss. Css property strings sometimes contain quotes, so we don't
want to escape them by default. For example:
background: url('http://example.com/marble.png');
I added a note in the JavaDoc that SafeCssProperties should only use
single quotes, and that the style attribute should use double quotes to
wrap the value. Eventually, we could update SafeHtmlTemplates to verify
that the style attribute wraps the value in double quotes.
http://gwt-code-reviews.appspot.com/1384801/diff/3003/user/test/com/google/gwt/safehtml/client/SafeHtmlTemplatesTest.java
File user/test/com/google/gwt/safehtml/client/SafeHtmlTemplatesTest.java
(right):
http://gwt-code-reviews.appspot.com/1384801/diff/3003/user/test/com/google/gwt/safehtml/client/SafeHtmlTemplatesTest.java#newcode80
user/test/com/google/gwt/safehtml/client/SafeHtmlTemplatesTest.java:80:
public void testTemplateWithCssAttribute() {
I added a some asserts to SafeCss to check for an end semi-colon, double
quotes, and brackets. The & character is valid because it can be used
in a URL:
background:url(http://url?image=123112&size=1024)
I added tests to SafeCssPropertiesString to test the assertions.
http://gwt-code-reviews.appspot.com/1384801/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Promotes Gin's AsyncProvider to GWT, along with a more general (issue1387801)
[+google-...@googlegroups.com] What dependency? DI is a pattern, not a commitment to a particular framework. That said, I agree that taking AsyncProvider from Gin is a bit presumptuous. I meant to include the gin community on this patch, adding them now. What do you think, folks? The goal here is to sever the dependency between com.google.gwt.inject.client and GWT RPC. On Mar 12, 2011 10:36 AM, wrote: > I support the move of the AsyncCallback! What is the purpose of adding > AsyncProvider to GWT's types? It seems only really useful if using > dependency injection (other usages are usually abuses :) and I don't see > why GWT would want that kind of dependency? > > http://gwt-code-reviews.appspot.com/1387801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] HasDataPresenter's SchedulerImpl coupling
Thats fine with me. You could also create a protected HasDataPresenter#scheduleCommand() method and just override it in your mock unit test version. Thanks, John LaBanca jlaba...@google.com On Sun, Mar 13, 2011 at 11:12 PM, Stephen Haberman < stephen.haber...@gmail.com> wrote: > Hi, > > I was attempting to reuse HasDataPresenter to drive some stub versions > of CellList/CellTable. I know HasDataPresenter is package private, but > I'm perfectly fine accepting the risk of it changing, as if it's > behavior/interface does change, then the behavior/interface of my stubs > would likely change to. > > However, HasDataPresenter currently has a single: > >Scheduler.get().scheduleFinally(pendingStateCommand); > > Call that leads to SchedulerImpl.INSTANCE, which is set by a static > GWT.create call. > > AFAICT, if I remove this Scheduler.get() reference (and replace it > with a Scheduler cstr parameter/field), HasDataPresenter will have no > more dependencies on anything being GWT.create'd, and I can happily > reuse it for unit tests. > > Please let me know if this is a bad idea, or unlikely to be accepted, > otherwise I'll plan on submitting a patch sometime soon. > > Thanks, > Stephen > > -- > http://groups.google.com/group/Google-Web-Toolkit-Contributors > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r9849 committed - creating the 2.3 release branch
Revision: 9849 Author: mrruss...@google.com Date: Mon Mar 14 08:33:12 2011 Log: creating the 2.3 release branch http://code.google.com/p/google-web-toolkit/source/detail?r=9849 Added: /releases/2.3 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix bug on IE where onload events don't fire for IFrames. (issue1294801)
http://gwt-code-reviews.appspot.com/1294801/diff/17001/user/src/com/google/gwt/user/client/ui/Frame.java File user/src/com/google/gwt/user/client/ui/Frame.java (right): http://gwt-code-reviews.appspot.com/1294801/diff/17001/user/src/com/google/gwt/user/client/ui/Frame.java#newcode78 user/src/com/google/gwt/user/client/ui/Frame.java:78: sinkEvents(Event.ONLOAD); Don't sink in the constructor unless its used by the Widget itself. addDomHandler will lazily sink the load event automatically. This is important because sinking events is expensive. http://gwt-code-reviews.appspot.com/1294801/diff/17001/user/src/com/google/gwt/user/client/ui/Frame.java#newcode110 user/src/com/google/gwt/user/client/ui/Frame.java:110: return addHandler(handler, LoadEvent.getType()); Use addDomHandler() for events that wrap native events. It will sink ONLOAD the first time it is called. http://gwt-code-reviews.appspot.com/1294801/diff/17001/user/test/com/google/gwt/dom/public-test/iframetest.html File user/test/com/google/gwt/dom/public-test/iframetest.html (right): http://gwt-code-reviews.appspot.com/1294801/diff/17001/user/test/com/google/gwt/dom/public-test/iframetest.html#newcode3 user/test/com/google/gwt/dom/public-test/iframetest.html:3: This doesn't look right http://gwt-code-reviews.appspot.com/1294801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix problem with user-agent based locale selection (issue1382803)
LGTM http://gwt-code-reviews.appspot.com/1382803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixing touch scrolling support for devices that support it natively. Our current touch support o... (issue1369809)
ping http://gwt-code-reviews.appspot.com/1369809/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Promotes Gin's AsyncProvider to GWT, along with a more general (issue1387801)
LGTM.
http://gwt-code-reviews.appspot.com/1387801/diff/5/user/src/com/google/gwt/core/client/AsyncProvider.java
File user/src/com/google/gwt/core/client/AsyncProvider.java (right):
http://gwt-code-reviews.appspot.com/1387801/diff/5/user/src/com/google/gwt/core/client/AsyncProvider.java#newcode21
user/src/com/google/gwt/core/client/AsyncProvider.java:21: * via {@link
AsyncCallback}. For example, the instance might be
via an {@link
http://gwt-code-reviews.appspot.com/1387801/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
