[gwt-contrib] Re: Rename RoletypeRole to Role and improve the javadoc. Subtypes of Role now (issue1799804)

2012-08-07 Thread skybrian
http://gwt-code-reviews.appspot.com/1799804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Added Aria roles and properties to the CellTree (issue1776803)

2012-08-07 Thread skybrian
This looks good, other than the corner case for a leaf that used to be a parent. http://gwt-code-reviews.appspot.com/1776803/diff/8001/user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java File user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java (right): http://gwt-

[gwt-contrib] Rename RoletypeRole to Role and improve the javadoc. Subtypes of Role now (issue1799804)

2012-08-03 Thread skybrian
Reviewers: atincheva, Description: Rename RoletypeRole to Role and improve the javadoc. Subtypes of Role now link to the ARIA specification. Please review this at http://gwt-code-reviews.appspot.com/1799804/ Affected files: M user/src/com/google/gwt/aria/client/AlertRole.java M user/src/co

[gwt-contrib] Rename aria.Role to aria.RoleImpl and make private. (The next step will be to (issue1802803)

2012-08-03 Thread skybrian
Reviewers: mdempsky, Description: Rename aria.Role to aria.RoleImpl and make private. (The next step will be to rename aria.RoletypeRole to aria.Role.) Rationale: this hides the implemention so that it's not in the public API, and we can change the implementation more freely in future GWT releas

[gwt-contrib] Re: Fixes soft permutation selection by converting the softPermutationId into an int. (issue1793806)

2012-08-01 Thread skybrian
LGTM http://gwt-code-reviews.appspot.com/1793806/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Increase a test timeout to prevent testOldMethods() from failing when (issue1793805)

2012-07-31 Thread skybrian
Reviewers: mdempsky, Description: Increase a test timeout to prevent testOldMethods() from failing when run using emma, Firefox, and JDK 7. (It would be interesting to know why this is slower, but I didn't track down the root cause.) Please review this at http://gwt-code-reviews.appspot.com/17

[gwt-contrib] Re: Fix incorrect SuggestBox documentation related to adding selection handlers. Fixes issue 4575. (issue1801803)

2012-07-31 Thread skybrian
LGTM http://gwt-code-reviews.appspot.com/1801803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Make ScriptInjectorTest less flaky by referencing locally-served test data. (issue1800803)

2012-07-31 Thread skybrian
LGTM http://gwt-code-reviews.appspot.com/1800803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Make ScriptInjectorTest less flaky by referencing locally-served test data. (issue1800803)

2012-07-30 Thread skybrian
http://gwt-code-reviews.appspot.com/1800803/diff/1/user/test/com/google/gwt/core/public/script_injector_test_absolute.js File user/test/com/google/gwt/core/public/script_injector_test_absolute.js (right): http://gwt-code-reviews.appspot.com/1800803/diff/1/user/test/com/google/gwt/core/public/scr

[gwt-contrib] Increase a timeout in JsonpRequestSuite to see if it fixes a flaky test. (issue1798804)

2012-07-30 Thread skybrian
Reviewers: cromwellian, Description: Increase a timeout in JsonpRequestSuite to see if it fixes a flaky test. (It seems to help when running the tests by hand, but it's hard to tell.) Please review this at http://gwt-code-reviews.appspot.com/1798804/ Affected files: M user/test/com/google/gw

[gwt-contrib] Re: Fix UiSuite to pass when test methods are run in a different order. (issue1795804)

2012-07-27 Thread skybrian
On 2012/07/27 07:48:14, cromwellian wrote: LGTM. Is this the only case? I would imagine there's probably many tests that were unknowingly dependent on the DOM being modified and left with persistent state between tests. I don't know. My guess is that most tests don't care that there's other

[gwt-contrib] Fix UiSuite to pass when test methods are run in a different order. (issue1795804)

2012-07-26 Thread skybrian
Reviewers: cromwellian, Description: Fix UiSuite to pass when test methods are run in a different order. The symptom was that many tests would fail, starting with RootPanelTest.testGetById(). I don't know the root cause, but it seems that leaving a RichTextArea with focus in the RootPanel screwe

[gwt-contrib] Re: Added Aria roles and properties to the CellTree (issue1776803)

2012-07-26 Thread skybrian
Thanks, that's gotten me reading the right code; before I couldn't even find the SelectionChangeListener and now I see that it's in HasDataPresenter. So for this CL, I think we just assume rendering happens when it needs to and update aria-selected (and other attributes) whenever a node gets re-r

[gwt-contrib] Re: Fix JSON escaping of unicode characters to work in JDK 7. (issue1796803)

2012-07-26 Thread skybrian
On 2012/07/26 07:02:08, tbroyer wrote: I'm not against this kind of change but IMO any deviation from the JSON and/or ECMAScript specs should be documented, or we risk removing them at a later time and break things again (that being said, I don't understand what's "broken" here and how this

[gwt-contrib] Re: A couple of little fixes to get nicer coverage results. (issue1797803)

2012-07-26 Thread skybrian
LGTM http://gwt-code-reviews.appspot.com/1797803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Fix JSON escaping of unicode characters to work in JDK 7. (issue1796803)

2012-07-25 Thread skybrian
Reviewers: acleung, Description: Fix JSON escaping of unicode characters to work in JDK 7. JDK 7 supports Unicode 6 and some characters changed: - zero-width-space is no longer a whitespace character - invisible-plus is new This caused JSON encoding tests to fail in HTMLUnit somehow, so escape

[gwt-contrib] Fix TestOracleMediatorFromByteCodeTest to work when run on JDK 7. (issue1794804)

2012-07-25 Thread skybrian
Reviewers: cromwellian, Description: Fix TestOracleMediatorFromByteCodeTest to work when run on JDK 7. The issue is that java.lang.Throwable has nested classes in JDK 7 but not JDK 6. So, change the assertion to allow any nested classes. Please review this at http://gwt-code-reviews.appspot.com

[gwt-contrib] Re: Added Aria roles and properties to the CellTree (issue1776803)

2012-07-25 Thread skybrian
Thanks John. On 2012/07/25 22:42:13, john.labanca wrote: As far as issue 7480, it sounds like a bug. Updating the SelectionModel should update the CellTree's keyboard selection. Well, the question is how to do this. I assume we install a SelectionChangeHandler. However, we don't know how ma

[gwt-contrib] Fix TreeMap and TreeSet emulation tests to pass with JDK 7. (issue1793803)

2012-07-24 Thread skybrian
Reviewers: cromwellian, Description: Fix TreeMap and TreeSet emulation tests to pass with JDK 7. No changes to prod mode. In JavaScript, we will keep the somewhat more lax behavior of JDK 6. Developers moving to JDK 7 who rely on the JDK 6 TreeMap/TreeSet behavior will see new exceptions in dev

[gwt-contrib] Re: Add "multiple" attribute to InputElement / FileUpload widget to enable the HTML5 multiple attribute. (issue1786803)

2012-07-24 Thread skybrian
I'd like to get the untested tests into the test suite, but not if they're broken or hanging. It sounds like a separate CL to me. http://gwt-code-reviews.appspot.com/1786803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Introduce a flag that explicitly turns coverage on/off (default off). (issue1786805)

2012-07-24 Thread skybrian
On 2012/07/24 23:19:54, cromwellian wrote: I don't understand why the dual flags, but I would not make it a proper compiler flag because there are some static clinit inits in compiler classes that need to know whether coverage is enabled when the classes are loaded. I was thinking it would

[gwt-contrib] Re: Introduce a flag that explicitly turns coverage on/off (default off). (issue1786805)

2012-07-24 Thread skybrian
http://gwt-code-reviews.appspot.com/1786805/diff/1/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java File dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java (right): http://gwt-code-reviews.appspot.com/1786805/diff/1/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaSc

[gwt-contrib] Re: Firefox 14 DevMode Plugin (issue1792803)

2012-07-24 Thread skybrian
Thanks Alan. http://gwt-code-reviews.appspot.com/1792803/diff/1/plugins/xpcom/JavaObject.h File plugins/xpcom/JavaObject.h (right): http://gwt-code-reviews.appspot.com/1792803/diff/1/plugins/xpcom/JavaObject.h#newcode55 plugins/xpcom/JavaObject.h:55: static void finalize(JSFreeOp* fop, JSObjec

[gwt-contrib] Re: Add compilation progress logging, visible in dev mode when log level set to TRACE. This is helpf... (issue1749803)

2012-07-24 Thread skybrian
LGTM http://gwt-code-reviews.appspot.com/1749803/diff/12001/dev/core/src/com/google/gwt/dev/javac/ProgressLogger.java File dev/core/src/com/google/gwt/dev/javac/ProgressLogger.java (right): http://gwt-code-reviews.appspot.com/1749803/diff/12001/dev/core/src/com/google/gwt/dev/javac/ProgressLog

[gwt-contrib] Super Dev Mode cleanup in preparation for doing more server-side HTML rendering (issue1791803)

2012-07-23 Thread skybrian
Reviewers: mdempsky, Description: Super Dev Mode cleanup in preparation for doing more server-side HTML rendering (no user-visible changes) - introduce HtmlWriter class - change log file rendering to use it - Move some code for handling generated file paths from SourceHandler to ModuleState Ple

[gwt-contrib] Re: Adds directory listings for the source files served by Super Dev Mode. (issue1770804)

2012-07-23 Thread skybrian
http://gwt-code-reviews.appspot.com/1770804/diff/1/dev/codeserver/java/com/google/gwt/dev/codeserver/SourceMap.java File dev/codeserver/java/com/google/gwt/dev/codeserver/SourceMap.java (right): http://gwt-code-reviews.appspot.com/1770804/diff/1/dev/codeserver/java/com/google/gwt/dev/codeserver/

[gwt-contrib] Re: Make DagePicker.getMonthSelector public. (issue1790803)

2012-07-23 Thread skybrian
It looks like if you subclass DatePicker, you will be able to call this method from your subclass. Will that work for you? For an arbitrary DatePicker, making this method public won't help much because the MonthSelector interface doesn't have any interesting methods. http://gwt-code-reviews.app

[gwt-contrib] Re: Added Aria roles and properties to the CellTree (issue1776803)

2012-07-23 Thread skybrian
The code for CellTree is rather odd. There is a keyboard selection policy with three values: DISABLED, ENABLED, and BOUND_TO_SELECTION. If the policy is ENABLED or BOUND_TO_SELECTION then the CSS styles for keyboard selection get updated, but the SelectionModel is only updated when the policy is

[gwt-contrib] Re: Add "multiple" attribute to InputElement / FileUpload widget to enable the HTML5 multiple attribute. (issue1786803)

2012-07-20 Thread skybrian
On 2012/07/20 20:47:26, heinberg wrote: Do you know how that works with unsupported attributes? Do they still get set and just do nothing? That's generally the case but I don't know about IE. I think think the best way to find out is to run the tests. There are four targets containing "DOMSuit

[gwt-contrib] Re: Add a new constructor to CellTree to specify defaultNodeSize. (issue1760804)

2012-07-20 Thread skybrian
LGTM http://gwt-code-reviews.appspot.com/1760804/diff/6001/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/1760804/diff/6001/user/src/com/google/gwt/user/cellview/client/Cell

[gwt-contrib] Re: Add "multiple" attribute to InputElement / FileUpload widget to enable the HTML5 multiple attribute. (issue1786803)

2012-07-20 Thread skybrian
On 2012/07/20 20:32:46, heinberg wrote: Sweet, thanks! Do you know how I can exclude the tests from running under IE9 and earlier? Not offhand, but we don't need to exclude them unless they break. These tests only check that the attribute gets set, right? http://gwt-code-reviews.appspot.com

[gwt-contrib] Re: Improved accessiblity of date picker. (issue1788803)

2012-07-20 Thread skybrian
LGTM (For the record, we're converting the cells from UIObject to Widget because the keyboard events don't seem to be bubbling up for some reason. If that's fixed then we can convert it back.) http://gwt-code-reviews.appspot.com/1788803/diff/1/user/src/com/google/gwt/user/datepicker/client/Cal

[gwt-contrib] Re: Add "multiple" attribute to InputElement / FileUpload widget to enable the HTML5 multiple attribute. (issue1786803)

2012-07-20 Thread skybrian
Okay, if it doesn't break anything and single-file uploads still work, I think we can add it with an appropriate warning. Something like: "This setting has no effect in some browsers, including Internet Explorer 9 and earlier." - Brian http://gwt-code-reviews.appspot.com/1786803/ -- http://gr

[gwt-contrib] Re: Add "multiple" attribute to InputElement / FileUpload widget to enable the HTML5 multiple attribute. (issue1786803)

2012-07-19 Thread skybrian
So one gotcha is that IE apparently doesn't support this attribute: http://www.w3schools.com/html5/att_input_multiple.asp I don't have a Windows box handy, but I did some web searches and apparently that was still true in IE 9. So I wonder if this is still useful for your use case? Historical

[gwt-contrib] Re: Add "multiple" attribute to InputElement / FileUpload widget to enable the HTML5 multiple attribute. (issue1786803)

2012-07-19 Thread skybrian
LGTM http://gwt-code-reviews.appspot.com/1786803/diff/1/user/test/com/google/gwt/dom/DOMSuite.java File user/test/com/google/gwt/dom/DOMSuite.java (right): http://gwt-code-reviews.appspot.com/1786803/diff/1/user/test/com/google/gwt/dom/DOMSuite.java#newcode47 user/test/com/google/gwt/dom/DOMSu

[gwt-contrib] Fixes Super Dev Mode to work for a GWT app that uses the PrecompileLinker (issue1783803)

2012-07-18 Thread skybrian
Reviewers: mdempsky, Description: Fixes Super Dev Mode to work for a GWT app that uses the PrecompileLinker with precompress.leave.originals set to false. We look for a gzip file and serve that instead. (Not implemented: uncompressing the file for web clients that don't support gzip compression.)

[gwt-contrib] Re: Allowing users to hit enter when focused on table header cell to sort a sortable column. Also ma... (issue1765804)

2012-07-17 Thread skybrian
LGTM http://gwt-code-reviews.appspot.com/1765804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Added support for draggable="true/false" attribute in ClippedImages. (issue1781803)

2012-07-16 Thread skybrian
LGTM http://gwt-code-reviews.appspot.com/1781803/diff/6001/user/test/com/google/gwt/user/client/ui/impl/ClippedImagePrototypeTest.java File user/test/com/google/gwt/user/client/ui/impl/ClippedImagePrototypeTest.java (right): http://gwt-code-reviews.appspot.com/1781803/diff/6001/user/test/com/g

[gwt-contrib] Re: Added support for draggable="true/false" attribute in ClippedImages. (issue1781803)

2012-07-16 Thread skybrian
Seems reasonable. Could you add a test in ClippedImagePrototypeTest? http://gwt-code-reviews.appspot.com/1781803/diff/1/user/src/com/google/gwt/user/client/ui/impl/ClippedImagePrototype.java File user/src/com/google/gwt/user/client/ui/impl/ClippedImagePrototype.java (right): http://gwt-code-re

[gwt-contrib] Re: Refactor coverage instrumentation, moving javascript to a separate file. (issue1780803)

2012-07-16 Thread skybrian
On 2012/07/16 20:04:42, isbadawi wrote: Since merge is only called in merge_coverage and merge_coverage is only called in the handler, they can be local functions like this. Oops! Somehow I misread this code. Sorry about that! LGTM http://gwt-code-reviews.appspot.com/1780803/ -- http://gr

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

2012-07-16 Thread skybrian
LGTM http://gwt-code-reviews.appspot.com/1735804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Refactor coverage instrumentation, moving javascript to a separate file. (issue1780803)

2012-07-16 Thread skybrian
http://gwt-code-reviews.appspot.com/1780803/diff/1/dev/core/src/com/google/gwt/dev/js/coverage.js File dev/core/src/com/google/gwt/dev/js/coverage.js (right): http://gwt-code-reviews.appspot.com/1780803/diff/1/dev/core/src/com/google/gwt/dev/js/coverage.js#newcode17 dev/core/src/com/google/gwt/d

[gwt-contrib] Re: Add instrumentation for collecting client-side code coverage. (issue1764803)

2012-07-13 Thread skybrian
LGTM http://gwt-code-reviews.appspot.com/1764803/diff/6003/dev/core/src/com/google/gwt/dev/js/BaselineCoverageGatherer.java File dev/core/src/com/google/gwt/dev/js/BaselineCoverageGatherer.java (right): http://gwt-code-reviews.appspot.com/1764803/diff/6003/dev/core/src/com/google/gwt/dev/js/Bas

[gwt-contrib] Re: Allowing users to hit enter when focused on table header cell to sort a sortable column. Also ma... (issue1765804)

2012-07-11 Thread skybrian
Seems reasonable. Could you write a test to make sure it continues to work? http://gwt-code-reviews.appspot.com/1765804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Adds directory listings for the source files served by Super Dev Mode. (issue1770804)

2012-07-11 Thread skybrian
Reviewers: acleung, Description: Adds directory listings for the source files served by Super Dev Mode. This makes it easier to browse source code when you're not using the Chrome debugger. There are two new page types: - A page for each module that lists all the directories containing at least o

[gwt-contrib] Re: Added Aria roles and properties to the CellTree (issue1776803)

2012-07-11 Thread skybrian
http://gwt-code-reviews.appspot.com/1776803/diff/1/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/1776803/diff/1/user/src/com/google/gwt/user/cellview/client/CellTree.java#new

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

2012-07-09 Thread skybrian
Also, in the official documentation, we don't put doctypes on module files. https://gwt-code-reviews.appspot.com/1772803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

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

2012-07-09 Thread skybrian
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 downloading DTD's, even if users put them in their gw

[gwt-contrib] Re: Include sources in gwt-codeserver.jar (issue1768804)

2012-06-30 Thread skybrian
LGTM http://gwt-code-reviews.appspot.com/1768804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Minor cleanup of Super Dev Mode: move some static methods to a separate class. (issue1767803)

2012-06-28 Thread skybrian
http://gwt-code-reviews.appspot.com/1767803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Minor cleanup of Super Dev Mode: move some static methods to a separate class. (issue1767803)

2012-06-28 Thread skybrian
Reviewers: acleung, Description: Minor cleanup of Super Dev Mode: move some static methods to a separate class. Please review this at http://gwt-code-reviews.appspot.com/1767803/ Affected files: A dev/codeserver/java/com/google/gwt/dev/codeserver/PageUtil.java M dev/codeserver/java/com/goo

[gwt-contrib] Convert dynatable sample to use GWT-RPC instead of DeRPC. (issue1759803)

2012-06-25 Thread skybrian
Reviewers: rdayal, Description: Convert dynatable sample to use GWT-RPC instead of DeRPC. Workaround for issue 7453. Please review this at http://gwt-code-reviews.appspot.com/1759803/ Affected files: M samples/dynatable/src/com/google/gwt/sample/dynatable/DynaTable.gwt.xml M samples/dyn

[gwt-contrib] Re: Add compilation progress logging, visible in dev mode when log level set to TRACE. This is helpf... (issue1749803)

2012-06-25 Thread skybrian
I don't have time to try it out right now, but here are a few things. http://gwt-code-reviews.appspot.com/1749803/diff/9001/dev/core/src/com/google/gwt/dev/javac/ProgressLogger.java File dev/core/src/com/google/gwt/dev/javac/ProgressLogger.java (right): http://gwt-code-reviews.appspot.com/1749

[gwt-contrib] Re: Fix unicode escaping in JSON Util. (issue1754803)

2012-06-21 Thread skybrian
On 2012/06/21 19:58:13, acleung wrote: LGTM. Could you also create a GWT Issue? http://gwt-code-reviews.appspot.com/1754803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Add compilation progress logging, visible in dev mode when log level set to TRACE. This is helpf... (issue1749803)

2012-06-21 Thread skybrian
Seems fine in principle. What does this look like in context, when there are also compile warnings or errors? It might be nice to log the total compilation time as well. http://gwt-code-reviews.appspot.com/1749803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Increase heap size for testing to prevent GC thrashing and OOMs when running usr/BigIntegerSuite. (issue1750803)

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

[gwt-contrib] Re: Make CoverageTest less dependent on execution order. (issue1741804)

2012-06-15 Thread skybrian
Looks good as far as it goes. http://gwt-code-reviews.appspot.com/1741804/diff/1/user/test/com/google/gwt/dev/jjs/test/CoverageTest.java File user/test/com/google/gwt/dev/jjs/test/CoverageTest.java (right): http://gwt-code-reviews.appspot.com/1741804/diff/1/user/test/com/google/gwt/dev/jjs/test

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

2012-06-15 Thread skybrian
LGTM. It looks like this was already fixed for the google build, so it's purely an open source change. https://gwt-code-reviews.appspot.com/1741803/ -- 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 skybrian
LGTM. I commented on some issues to fix in later patches. http://gwt-code-reviews.appspot.com/1727808/diff/16001/distro-source/build.xml File distro-source/build.xml (right): http://gwt-code-reviews.appspot.com/1727808/diff/16001/distro-source/build.xml#newcode39 distro-source/build.xml:39: I

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

2012-06-14 Thread skybrian
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 about 4:30. https://gwt-code-reviews.appspot.com/1727808/ -- http://groups.google.co

[gwt-contrib] Fix Super Dev Mode's code server to launch on localhost by default. (issue1740803)

2012-06-14 Thread skybrian
Reviewers: acleung, Description: Fix Super Dev Mode's code server to launch on localhost by default. (There were comments about this but it wasn't actually true.) Add -bindAddress option to override this. In particular, -bindAddress 0.0.0.0 restores previous behavior. Please review this at http

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

2012-06-14 Thread skybrian
https://gwt-code-reviews.appspot.com/1727808/diff/8001/build.xml File build.xml (right): https://gwt-code-reviews.appspot.com/1727808/diff/8001/build.xml#newcode41 build.xml:41: I asked Ray about this. Elemental is experimental and its build process is rather interesting (and probably not very

[gwt-contrib] Re: Removes execution order dependency on EnumOrdinalizerTest (issue1731806)

2012-06-14 Thread skybrian
LGTM http://gwt-code-reviews.appspot.com/1731806/diff/1/dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java File dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java (right): http://gwt-code-reviews.appspot.com/1731806/diff/1/dev/core/test/com/google/gwt/dev/jjs/imp

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

2012-06-14 Thread skybrian
LGTM. I'll submit this. On 2012/06/14 07:32:40, tbroyer wrote: because gwt-user has no declared dependency on gwt-dev (this might be a mistake, or not) I'm guessing this is because we don't want GWT user code to have dependencies on classes that should only be used in the compiler. https://

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

2012-06-13 Thread skybrian
Reviewers: cromwellian, Description: Include json-1.5.jar in gwt-dev.jar. JSON is needed by the Closure compiler and also to generate source maps. Fixes issue 7397 Please review this at http://gwt-code-reviews.appspot.com/1732804/ Affected files: M dev/build.xml M dev/codeserver/build.xml

[gwt-contrib] Re: Comment out an invalid test in TreeMapTest. (issue1735805)

2012-06-13 Thread skybrian
LGTM It seems like we should also change the web implementation to behave like JDK 7. But that can wait. http://gwt-code-reviews.appspot.com/1735805/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

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

2012-06-13 Thread skybrian
I'm getting failing tests because JSON is gone from gwt-user.jar and the requestfactory jars. I could add the dependency internal to google, but I think we might still be trying to do too much at once - this is looking more like churn than an actual improvement. To fix the compiler and close issu

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

2012-06-13 Thread skybrian
LGTM. I can understand the code now and I'm basically okay with it; the rest is nitpicks. http://gwt-code-reviews.appspot.com/1727807/diff/15003/user/src/com/google/gwt/editor/client/impl/SimpleViolation.java File user/src/com/google/gwt/editor/client/impl/SimpleViolation.java (right): http://

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

2012-06-13 Thread skybrian
Okay, seems fine for now. I'm going to commit this. https://gwt-code-reviews.appspot.com/1731804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

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

2012-06-13 Thread skybrian
I mostly looked over the build process. (Since this is experimental, I suppose it can all be done later.) http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/META-INF/MANIFEST.MF File elemental/META-INF/MANIFEST.MF (right): http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/M

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

2012-06-12 Thread skybrian
http://gwt-code-reviews.appspot.com/1735804/diff/1/user/src/com/google/gwt/resources/css/SubstitutionReplacer.java File user/src/com/google/gwt/resources/css/SubstitutionReplacer.java (right): http://gwt-code-reviews.appspot.com/1735804/diff/1/user/src/com/google/gwt/resources/css/SubstitutionRe

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

2012-06-12 Thread skybrian
LGTM https://gwt-code-reviews.appspot.com/1729805/diff/1/user/test/com/google/web/bindery/requestfactory/apt/EntityProxyCheckDomainMapping.java File user/test/com/google/web/bindery/requestfactory/apt/EntityProxyCheckDomainMapping.java (right): https://gwt-code-reviews.appspot.com/1729805/diff/

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

2012-06-12 Thread skybrian
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): http://gwt-code-reviews.appspot.com/1727807/diff/7/user/src/com/google/gwt/editor/client/impl/SimpleVio

[gwt-contrib] Re: Make CodeSplitter2 outputs a reasonable SOYC. (issue1726803)

2012-06-11 Thread skybrian
I don't understand this code, but I wonder if there is any way to write a smoke test for soyc? http://gwt-code-reviews.appspot.com/1726803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

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

2012-06-11 Thread skybrian
http://gwt-code-reviews.appspot.com/1727807/diff/8001/user/src/com/google/gwt/editor/client/impl/DelegateMap.java File user/src/com/google/gwt/editor/client/impl/DelegateMap.java (right): http://gwt-code-reviews.appspot.com/1727807/diff/8001/user/src/com/google/gwt/editor/client/impl/DelegateMap

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

2012-06-11 Thread skybrian
http://gwt-code-reviews.appspot.com/1727807/diff/8001/user/src/com/google/gwt/editor/client/impl/DelegateMap.java File user/src/com/google/gwt/editor/client/impl/DelegateMap.java (right): http://gwt-code-reviews.appspot.com/1727807/diff/8001/user/src/com/google/gwt/editor/client/impl/DelegateMap

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

2012-06-11 Thread skybrian
http://gwt-code-reviews.appspot.com/1727803/diff/4003/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/4003/user/src/com/google/gwt/resources/css/RtlVisitor.java#newcode46 us

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

2012-06-08 Thread skybrian
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 validation support ought to be split out of gwt-user into

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

2012-06-06 Thread skybrian
LGTM. Thanks! https://gwt-code-reviews.appspot.com/1734803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

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

2012-06-05 Thread skybrian
Ray, you can see the diffs by looking at internal code review. I updated a bunch of javadoc, tweaked Recompiler to deal with running with a different linker, and changed the regexps in WebServer to be slightly tighter and a bit more readable. But sure, basically the same. http://gwt-code-review

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

2012-06-05 Thread skybrian
http://gwt-code-reviews.appspot.com/1727804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

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

2012-06-04 Thread skybrian
Reviewers: cromwellian, Description: Move Super Dev Mode to the open source repository. Includes a simple "demo" target that works with the "Hello" sample app. I've also made some linker options configurable, as necessary to make this work. Review by: cromwell...@google.com Please review this at

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

2012-05-30 Thread skybrian
LGTM (assuming tests still pass) http://gwt-code-reviews.appspot.com/1601806/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

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

2012-05-30 Thread skybrian
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. This isn't true with this CL, as the point of t

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

2012-05-29 Thread skybrian
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 Intern/Manager/Department should be moved to top

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

2012-05-29 Thread skybrian
On 2012/05/26 23:09:03, tbroyer wrote: (the use OperationMessage here can already be seen as a "hack", kind of, because all we need is a Map for the storage, and a String for the version; so using another field in some "diverted" way is not really a problem I think) Oh yuck. Yeah, OperationMess

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

2012-05-25 Thread skybrian
LGTM. (Some optional nitpicks.) It seems a little odd that you'd want to do this. I'd expect that the subtype (Manager or Intern) would have additional fields to edit, so you wouldn't be able to just reuse the editor for the parent type. But it should still be allowed. http://gwt-code-reviews.

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

2012-05-23 Thread skybrian
/bindery/requestfactory/server/SimpleBar.java#newcode95 user/test/com/google/web/bindery/requestfactory/server/SimpleBar.java:95: public static SimpleBar returnFirst(List list) { On 2012/05/24 00:28:11, tbroyer wrote: On 2012/05/23 18:05:24, skybrian wrote: > It's weird that this method isn't

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

2012-05-23 Thread skybrian
LGTM http://gwt-code-reviews.appspot.com/1712804/diff/1/samples/showcase/src/com/google/gwt/sample/showcase/client/content/lists/CwTree.java File samples/showcase/src/com/google/gwt/sample/showcase/client/content/lists/CwTree.java (right): http://gwt-code-reviews.appspot.com/1712804/diff/1/sam

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

2012-05-23 Thread skybrian
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): http://gwt-code-reviews.appspot.com/1622803/diff/6001/user/src/com/google/

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

2012-05-23 Thread skybrian
I'm fine with the production code. This is mostly nitpicks, except that we need to make sure that the code in RequestPayloadTest is actually run. http://gwt-code-reviews.appspot.com/1601806/diff/45014/user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java File us

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

2012-05-18 Thread skybrian
I've read about Places and PlaceTokenizers and I'm a bit confused. It looks like the generated PlaceHistoryMapper does a bunch of instanceof checks on a passed-in Place to figure out which tokenizer to use to create the token. So if we have multiple PlaceTokenizers with the same Place type paramet

[gwt-contrib] Re: Using SafeStyles in the ButtonCellBase Templates to avoid the compiler warnings against using St... (issue1707804)

2012-05-17 Thread skybrian
LGTM http://gwt-code-reviews.appspot.com/1707804/diff/1/user/src/com/google/gwt/cell/client/ButtonCellBase.java File user/src/com/google/gwt/cell/client/ButtonCellBase.java (right): http://gwt-code-reviews.appspot.com/1707804/diff/1/user/src/com/google/gwt/cell/client/ButtonCellBase.java#newco

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

2012-05-17 Thread skybrian
Okay, yeah, a TODO is fine as it looks kind of difficult. I see now that we are calling ValueProxy.equals() which calls deepEquals(). Worse, when AutoBeanUtils.diff() is called with a ValueProxy, it looks like we will be doing a deep equality check twice. (For the "fast" check and again for each p

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

2012-05-17 Thread skybrian
Oops, I had started an email about the revert but apparently never sent it. Sorry about that! I might need some background about the editor framework... https://gwt-code-reviews.appspot.com/1664803/diff/4002/user/src/com/google/gwt/editor/client/adapters/ListEditor.java File user/src/com/googl

[gwt-contrib] Re: Fixing a bug in MultiSelectionModel where it does not respect the order of selection when select... (issue1707803)

2012-05-15 Thread skybrian
LGTM http://gwt-code-reviews.appspot.com/1707803/diff/6001/user/src/com/google/gwt/view/client/OrderedMultiSelectionModel.java File user/src/com/google/gwt/view/client/OrderedMultiSelectionModel.java (right): http://gwt-code-reviews.appspot.com/1707803/diff/6001/user/src/com/google/gwt/view/cli

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

2012-05-15 Thread skybrian
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): https://gwt-code-reviews.appspot.com/1674804/diff/1/user/src/com/google/gwt/place/rebind

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

2012-05-15 Thread skybrian
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 don't care what the diffs are and can return ea

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

2012-05-15 Thread skybrian
Okay, so "immutable" here just means "not editable", aka not checked out for edit, in a source control sense. http://gwt-code-reviews.appspot.com/1601806/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

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

2012-05-15 Thread skybrian
This may be a naive question, but how are immutable entities supposed to work with the RequestContext lifecycle? I think the more normal approach is something like this: - fetch an entity using a RequestContext - start editing it - commit it back, using the same RequestContext Alternately you c

<    1   2   3   >