Reviewers: rjrjr,
Message:
This looks great to me, Alex. And I think you're doing as right a thing
as you can WRT Chrome. Just a couple of nits noted below.
http://codereview.appspot.com/8696/diff/1/4
File user/src/com/google/gwt/user/client/ui/impl/HyperlinkImplIE.java
(right):
http
Seems like we're missing a method from the Format interface?
http://gwt-code-reviews.appspot.com/1201/diff/12/14
File user/src/com/google/gwt/user/datepicker/client/DateBox.java
(right):
http://gwt-code-reviews.appspot.com/1201/diff/12/14#newcode173
Line 173: Date parse(DateBox dateBox, String
LGTM
http://gwt-code-reviews.appspot.com/1401
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
http://gwt-code-reviews.appspot.com/2204/diff/1/2
File user/src/com/google/gwt/user/client/ui/PopupPanel.java (right):
http://gwt-code-reviews.appspot.com/2204/diff/1/2#newcode787
Line 787: DOM.addEventPreview(this);
Seems like you should document here (and above in Hide) that you're
using both
Reviewers: ecc, jgw,
Description:
This patch provides access to the underlying InputElement's value
property, and deprecates the old isChecked / setChecked methods that are
redundant with those implemented for HasValue.
In the process, I converted CheckBox to use the new Element methods
instead
http://gwt-code-reviews.appspot.com/2205/diff/1/24
File user/src/com/google/gwt/user/client/ui/Widget.java (left):
http://gwt-code-reviews.appspot.com/2205/diff/1/24#oldcode53
Line 53: public final HandlerManager getHandlers() {
On 2009/01/15 14:53:37, ecc wrote:
I think that would be a much
LGTM
http://gwt-code-reviews.appspot.com/2007
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
Reviewers: jgw, ecc,
Description:
This patch gets rid of the unused isEventHandled call on the new
HasHandlers interface. It is meant to be applied to the releases/1.6
branch.
Please review this at http://gwt-code-reviews.appspot.com/2209
Affected files:
Committed to releases/1.6 @r4509
On 2009/01/21 21:46:53, jgw wrote:
On 2009/01/21 19:04:33, rjrjr wrote:
LGTM. I can't think of any use case that really justifies adding this
extra
complexity to the HasHandlers interface (certainly not one that
couldn't be
easily worked around). The fact
Reviewers: jgw,
Description:
Updates 1.6 branch-info.txt to reflect today's merge
(490:4497,4498:4511). () to be updated when merge lands.
Note that --accept=postpone has been dropped. svn says there is no such
thing.
Please review this at http://gwt-code-reviews.appspot.com/2011
Affected
Reviewers: jgw,
Description:
This change restores TextBoxBase's implementation of
HasChangeEventHandlers
We recently removed TextBox's implementation of HasChangeEventHandlers,
reasoning that it was redundant with its implementation of
HasValueChangeString.
As I struggle to integrate this
On 2009/01/29 17:37:23, jlabanca wrote:
LGTM
Thanks, submitted @r4583
http://gwt-code-reviews.appspot.com/2401
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
LGTM!
http://gwt-code-reviews.appspot.com/2804
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
Reviewers: spoon, jaimeyap,
Message:
Is there really no test coverage for this?
Description:
http://code.google.com/p/google-web-toolkit/issues/detail?id=3277
Please review this at http://gwt-code-reviews.appspot.com/4802
Affected files:
Committed @r4708
http://gwt-code-reviews.appspot.com/4804
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
LGTM
http://gwt-code-reviews.appspot.com/7801
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
Reviewers: bruce,
Description:
GWT issue 434
Please review this at http://gwt-code-reviews.appspot.com/7803
Affected files:
user/src/com/google/gwt/user/client/ui/MenuItem.java
Index: user/src/com/google/gwt/user/client/ui/MenuItem.java
That was kind of terse, wasn't it?
This is a one line javadoc fix to eradicate a confusing lie in MenuItem.
http://gwt-code-reviews.appspot.com/7803
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Committed revision 4880.
http://gwt-code-reviews.appspot.com/7803
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
Reviewers: robertvawter,
Description:
When StackTraceElement became emulated and therefore useful, it stopped
being serializable. This patch introduces a custom field serializer for
it, and extends existing tests to cover it.
Please review this at http://gwt-code-reviews.appspot.com/9802
Reviewers: jgw,
Description:
Fix for issue 3454, RadioButtons fire ValueChangeEvents when clicked
even when value does not change
http://code.google.com/p/google-web-toolkit/issues/detail?id=3454
But it ain't working yet. Joel,
RadioButtonTest#testValueChangeViaLabelClick fails, though its
http://gwt-code-reviews.appspot.com/11801/diff/1/2
File user/src/com/google/gwt/user/client/ui/CheckBox.java (right):
http://gwt-code-reviews.appspot.com/11801/diff/1/2#newcode142
Line 142: }
Nope, but a nice try. What I actually had to do was extend the override
of Widget#sinkEvents to apply
Reviewers: jgw,
Description:
http://code.google.com/p/google-web-toolkit/issues/detail?id=3464
Fixes problem where if you try to create a mock instance of DateBox
(e.g. via easymock classextension), you fail with an NPE in
LocaleInfo.ensureDateTimeConstants. Extends ClassInitTest to cover date
Reviewers: jgw,
Description:
Addresses issue 3427, Need to improve @deprecated javadocs for new event
handler system
Please review this at http://gwt-code-reviews.appspot.com/12806
Affected files:
user/src/com/google/gwt/user/client/ui/ChangeListener.java
Reviewers: jlabanca,
Description:
This makes DisclosurePanelImagesRTL into a public interface, to allow
apps to combine it into a larger ImageBundle to be passed to the
DisclosurePanel(ImageBundle) constructor.
Please review this at http://gwt-code-reviews.appspot.com/12807
Affected files:
Reviewers: jlabanca,
Description:
This patch makes MenuBar#selectItem public, with an eye toward allowing
keyboard menu control.
Please review this at http://gwt-code-reviews.appspot.com/13801
Affected files:
user/src/com/google/gwt/user/client/ui/MenuBar.java
http://gwt-code-reviews.appspot.com/13802/diff/1/13
File
samples/styleloader/src/com/google/gwt/sample/styleloader/client/StyleLoader.java
(right):
http://gwt-code-reviews.appspot.com/13802/diff/1/13#newcode34
Line 34: public class StyleLoader implements EntryPoint {
Presuming we go straight to
http://gwt-code-reviews.appspot.com/15802
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
With my stupid question about the test answered offline, LGTM
http://gwt-code-reviews.appspot.com/15802
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
http://gwt-code-reviews.appspot.com/15803/diff/1/4
File user/src/com/google/gwt/dom/client/StyleInjector.java (right):
http://gwt-code-reviews.appspot.com/15803/diff/1/4#newcode42
Line 42: return injectStyleSheet(contents);
You're ignoring element. How can you get away with that?
Your public
LGTM
Some nits, do with them what you will and commit this puppy.
http://gwt-code-reviews.appspot.com/15803/diff/4001/4004
File user/src/com/google/gwt/dom/client/StyleInjector.java (right):
http://gwt-code-reviews.appspot.com/15803/diff/4001/4004#newcode35
Line 35: head).getItem(0));
line
LGTM
And I agree with putting this on the 1.6 branch, but it's Bruce's call
http://gwt-code-reviews.appspot.com/17801
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
Reviewers: jlabanca,
Description:
Extends ClassInitTest (the mockability test) to ensure the mockability
of classes in com.google.gwt.dom.client, and fixes a couple of spots
where they weren't.
Please review this at http://gwt-code-reviews.appspot.com/20803
Affected files:
On 2009/04/16 21:42:02, jlabanca wrote:
LGTM
Holding off on this. The JSOs are full of final methods, and so not
mockable. Need to think about how to get interfaces involved, and if
it's worth the bother.
http://gwt-code-reviews.appspot.com/20803
http://gwt-code-reviews.appspot.com/25801/diff/1/3
File user/src/com/google/gwt/event/shared/HandlerManager.java (right):
http://gwt-code-reviews.appspot.com/25801/diff/1/3#newcode101
Line 101: }
Could be a little simpler--you don't really need an else case:
boolean result = false;
if (l !=
Reviewers: jlabanca,
Description:
Fixes issue 3586
Please review this at http://gwt-code-reviews.appspot.com/29801
Affected files:
user/src/com/google/gwt/user/client/ui/UIObject.java
user/test/com/google/gwt/user/client/ui/UIObjectTest.java
Index:
LGTM
Nice. I like the test in particular. Hurray for fake events! But that
said, did you run the test on IE? I've been bitten by its event handling
lately.
http://gwt-code-reviews.appspot.com/32801
--~--~-~--~~~---~--~~
Reviewers: jlabanca,
Description:
CheckBoxes were sending two click events when their labels were clicked,
due to trickery going on to send ValueChangeEvents only when
appropriate.
I've moved all the trickery down to RadioButton, the only place it is
actually needed, and filter out click events
LGTM
http://gwt-code-reviews.appspot.com/33805
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
LGTM
http://gwt-code-reviews.appspot.com/33806
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
http://gwt-code-reviews.appspot.com/33807/diff/1/3
File user/src/com/google/gwt/user/client/ui/HTMLTable.java (right):
http://gwt-code-reviews.appspot.com/33807/diff/1/3#newcode728
Line 728: cleanCell(row, col, clearInnerHTML);
Rather than relying on the existing, one by one clearInnerHTML
LGTM
http://gwt-code-reviews.appspot.com/33810
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
LGTM
http://gwt-code-reviews.appspot.com/33812
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
http://gwt-code-reviews.appspot.com/33823/diff/1/3
File user/src/com/google/gwt/dom/client/InputElement.java (right):
http://gwt-code-reviews.appspot.com/33823/diff/1/3#newcode234
Line 234: return this.readOnly;
Shouldn't you coerce this as well, then? And the various other boolean
returns in
LGTM
with one caveat:
http://gwt-code-reviews.appspot.com/33823/diff/1007/1010
File user/src/com/google/gwt/dom/client/TextAreaElement.java (right):
http://gwt-code-reviews.appspot.com/33823/diff/1007/1010#newcode109
Line 109: return this.readOnly;
gotcha
Reviewers: jgw, jlabanca,
Message:
LGTM
Please review this at http://gwt-code-reviews.appspot.com/33820
Affected files:
M user/src/com/google/gwt/event/dom/client/MouseWheelEvent.java
Index: user/src/com/google/gwt/event/dom/client/MouseWheelEvent.java
http://gwt-code-reviews.appspot.com/33827/diff/1/12
File user/src/com/google/gwt/resources/client/CssResource.java (right):
http://gwt-code-reviews.appspot.com/33827/diff/1/12#newcode229
Line 229: * recommended as the default behavior for CssResources.
hindsightShould have made it the default
Reviewers: scottb,
Description:
Fix for
http://code.google.com/p/google-web-toolkit/issues/detail?id=3679
Please review this at http://gwt-code-reviews.appspot.com/34817
Affected files:
user/src/com/google/gwt/user/client/ui/RadioButton.java
Index:
http://gwt-code-reviews.appspot.com/34824/diff/1/3
File user/src/com/google/gwt/resources/rg/CssResourceGenerator.java
(right):
http://gwt-code-reviews.appspot.com/34824/diff/1/3#newcode221
Line 221: + using an @external declaration for unobfuscated classes.);
...for obfuscated classes, or
LGTM
http://gwt-code-reviews.appspot.com/34824
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
Reviewers: jlabanca,
Description:
John, if you have a moment...
Please review this at http://gwt-code-reviews.appspot.com/33844
Affected files:
reference/code-museum/src/com/google/gwt/museum/client/defaultmuseum/DefaultMuseum.java
[+ contrib]
http://gwt-code-reviews.appspot.com/39802
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
LGTM
http://gwt-code-reviews.appspot.com/47811
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
Reviewers: scottb,
Description:
Brian Stoler cooked up this dynamic proxy factory to allow JUnit tests
of objects that rely on generated Messages classes.
Is this the right package for it? GWTMockUtilities live here so it
seemed right.
What suite can I wire the unit test into?
Please review
Forgot about this during my vacation. In it goes.
Committed revision 5694.
On 2009/06/12 21:22:50, jgw wrote:
On 2009/06/12 21:20:50, jgw wrote:
Ray:
That won't really protect anyone from anything, as they could just
call
DOM.createElement(bugger) and hand that in. Also, accepting an
Committed revision 5711.
On 2009/07/09 00:02:01, Ray Ryan wrote:
Forgot about this during my vacation. In it goes.
Committed revision 5694.
On 2009/06/12 21:22:50, jgw wrote:
On 2009/06/12 21:20:50, jgw wrote:
Ray:
That won't really protect anyone from anything, as they could just
LGTM
http://gwt-code-reviews.appspot.com/47812
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
Reviewers: amitmanjhi,
Description:
Fix bad import order in TestSetValidator.java, and update gwt.import
order to ensure it stays fixed. Also fix param type warnings.
Please review this at http://gwt-code-reviews.appspot.com/51803
Affected files:
eclipse/settings/code-style/gwt.importorder
Reviewers: ,
Description:
I'm adding a few new libs to svn/tools, to allow use of easyMock in some
tests that are slated to be comitted soon.
Note that no production code leans on these yet, although I expect that
will change as we provide better test utilities for GWT users. For now
they're
Reviewers: jgw,
Description:
Hey Joel, remember this? I had to roll it back because it didn't work on
IE as it was before--IE is finicky about just where and how you write to
innerHTML. Can you re-review this robustificated version?
Please review this at
Reviewers: jlabanca,
Description:
Adds a warning to the javadoc for Event#addNativePreviewHandler
explaining its cross-app limitations.
Please review this at http://gwt-code-reviews.appspot.com/51821
Affected files:
user/src/com/google/gwt/user/client/Event.java
Index:
http://gwt-code-reviews.appspot.com/51825/diff/1/5
File user/src/com/google/gwt/user/tools/AppHtml.htmlsrc (right):
http://gwt-code-reviews.appspot.com/51825/diff/1/5#newcode41
Line 41: !-- RECOMMENDED if you web app will not function without
JavaScript enabled --
if you - if your
Reviewers: jgw,
Description:
Introduces UiBinder
http://code.google.com/p/google-web-toolkit-incubator/wiki/UiBinder
The actual source code has been through thorough code review over the
last year+ of use in various Google projects, including the new AdWords
UI and Wave. Feedback and criticism
On 2009/08/04 17:44:38, Ray Ryan wrote:
A question for the group: the stuff under rebind and parsers should not
be considered public API, it's just not ready for that. Is javadoc to
that effect enough of a deterrent? (Although I suppose the fact that you
can't actually make your own parsers and
http://gwt-code-reviews.appspot.com/51831/diff/1/2
File eclipse/user/.classpath (right):
http://gwt-code-reviews.appspot.com/51831/diff/1/2#newcode19
Line 19: classpathentry kind=var
path=GWT_TOOLS/lib/easymock/easymockclassextension.jar
On 2009/08/10 14:37:10, jgw wrote:
Does this address the problems Scott noted in the issue tracker, or did
you decide they're livable? Is there a test we can extend to cover this
case if not?
Quoth he:
I tried a few variations of adding/inserting an already attached child.
I found
one that
LGTM
http://gwt-code-reviews.appspot.com/57810
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
In the spirit of no good deed goes unpunished, can I talk you into a)
eradicating the tab characters in all of these samples; b) replacing
your message markup with the following and; c) revisiting your old patch
and do the same?
div style=width:22em; position:absolute; left: 50%;
LGTM
http://gwt-code-reviews.appspot.com/61803
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
This is a nice cleanup, but I have questions about your annotation
search
http://gwt-code-reviews.appspot.com/61802/diff/1011/18
File user/src/com/google/gwt/resources/ext/ResourceGeneratorUtil.java
(right):
http://gwt-code-reviews.appspot.com/61802/diff/1011/18#newcode135
Line 135: public
LGTM w/a couple of javadoc tweaks.
http://gwt-code-reviews.appspot.com/61802/diff/28/1039
File dev/core/src/com/google/gwt/core/ext/typeinfo/JClassType.java
(right):
http://gwt-code-reviews.appspot.com/61802/diff/28/1039#newcode368
Line 368: * which this type is assignable. Annotations present
LGTM
http://gwt-code-reviews.appspot.com/61807
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
Reviewers: ,
Description:
This is an internal contribution that extends FakeMessagesMaker to work
with Constants as well. I'll submit it if the c-build every turns green
again.
Please review this at http://gwt-code-reviews.appspot.com/61808
Affected files:
If we really need this to be an optional mechanism, make it opt out:
@Singleton(false)
and if the annotation is not provided, the static treatment is generated
http://gwt-code-reviews.appspot.com/56804
--~--~-~--~~~---~--~~
http://gwt-code-reviews.appspot.com/56804/diff/1005/7
File user/src/com/google/gwt/user/client/AsyncProxy.java (right):
http://gwt-code-reviews.appspot.com/56804/diff/1005/7#newcode54
Line 54: * private IFoo fooOrProxy = GWT.create(FooProxy.class);
static
Why not just create the static field
LGTM
http://gwt-code-reviews.appspot.com/61814
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
LGTM
http://gwt-code-reviews.appspot.com/62806
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
Reviewers: jgw,
Message:
Joel, if you would be so kind.
This is a prerequisite for
http://code.google.com/p/google-web-toolkit/issues/detail?id=3984,
ClientBundle via ui.xml.
Please review this at http://gwt-code-reviews.appspot.com/61817
Affected files:
M
Just updated this with checkstyle fixes.
http://gwt-code-reviews.appspot.com/61817
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
On 2009/08/28 17:57:29, jgw wrote:
(Ray and I just sat down together and went over the design in detail)
LGTM.
Committed at r6059
http://gwt-code-reviews.appspot.com/61817
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2009/08/20 03:03:32, Ray Ryan wrote:
Committed r6063
http://gwt-code-reviews.appspot.com/61808
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
On 2009/09/01 18:54:10, jgw wrote:
On 2009/08/31 21:24:06, Ray Ryan wrote:
Joel, can you take this review?
LGTM. Nice to see it all cleaned up like that.
Committed at r6062
http://gwt-code-reviews.appspot.com/62810
--~--~-~--~~~---~--~~
On 2009/09/01 22:18:35, Ray Ryan wrote:
On 2009/08/20 03:03:32, Ray Ryan wrote:
Committed r6063
Aaaand rolled back at r6067 before it can break the build.
http://gwt-code-reviews.appspot.com/61808
--~--~-~--~~~---~--~~
Reviewers: jgw,
Message:
Lays out the groundwork for
http://code.google.com/p/google-web-toolkit/issues/detail?id=3984
ui:style source=foo.css type=com.me.and.MyCss / now works. It
implies a
CssResource of type MyCss, provided by file foo.css, accessible as if it
had
ui:field=style. We generate
Okay, name - field was trivial, done.
http://gwt-code-reviews.appspot.com/64801
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
Thanks. Updated with some checkstyle fixes. Running tests, should submit
soon.
http://gwt-code-reviews.appspot.com/64801
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
Lex has volunteered to take the review
http://gwt-code-reviews.appspot.com/65805
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
Reviewers: Lex,
Message:
We were double escaping the contents of placeholders inside HTML
messages. E.g.,
p
ui:msgI would 'like' a
span ui:field='foo''single'/span quote
/ui:msg
/p
rendered as:
I would 'like' a ''single'' quote
There was already a mechanism in place
On 2009/09/09 16:53:02, Ray Ryan wrote:
I don't understand how this fixes anything. What's the actual test
failure?
But this strikes me as a bug in HTMLUnit. Browsers are converting
42.0px to 42px, and HTMLUnit isn't. Can we get the fix made where it
belongs?
On Wed, Sep 9, 2009 at 10:53 AM, Frank Lin f...@google.com wrote:
I would argue that is also our code to fix. In Style.java, we
implicitly change an int to a double. So in the case, 42 was changed
to 42.0 and we were relied on browser to correct it for us. In the
case of passing a real double
On 2009/09/10 19:54:05, Ray Ryan wrote:
Committed r6114
http://gwt-code-reviews.appspot.com/64812
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
On 2009/09/14 20:23:31, jlabanca wrote:
So the UncaughtExceptionHandler violates finally? Isn't that a pretty
fundamental problem?
http://gwt-code-reviews.appspot.com/64815
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
You've told Rietveld that this is a patch on gwt, note incubator. Can
you fix that?
http://gwt-code-reviews.appspot.com/68802
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
If Bob and Kelly haven't objected to this by, say, lunch time tomorrow,
can we get it submitted? This issue continues to bite people, and it
would be nice to see it fixed before MS1 freezes RSN.
http://gwt-code-reviews.appspot.com/56807
--~--~-~--~~~---~--~~
com.google.gwt.uibinder.rebind.model.OwnerField;
* document.
*/
public class UiBinderParser {
+ private static final String FIELD_ATTRIBUTE = field;
+ private static final String SOURCE_ATTRIBUTE = src;
+
// TODO(rjrjr) Make all the ElementParsers receive their dependencies via
// constructor like this one does
I'm still reviewing, but I thought I should get this AttributeParser
issue in front of you early.
http://gwt-code-reviews.appspot.com/68805/diff/1/24
File user/src/com/google/gwt/uibinder/parsers/DockLayoutPanelParser.java
(right):
http://gwt-code-reviews.appspot.com/68805/diff/1/24#newcode68
Still LGTM
I mean one rethrow nit below HERE
http://gwt-code-reviews.appspot.com/68805/diff/1/24
File user/src/com/google/gwt/uibinder/parsers/DockLayoutPanelParser.java
(right):
http://gwt-code-reviews.appspot.com/68805/diff/1/24#newcode115
Line 115: writer.die(Unexpected: Unable to find %s
Reviewers: jgw,
Message:
Joel, this is to change the served location of xhtml.ent
after discussion with Andrew
Please review this at http://gwt-code-reviews.appspot.com/72801
Affected files:
M user/src/com/google/gwt/uibinder/rebind/GwtResourceEntityResolver.java
M
Can you look again? One of my unit tests was passing vacuously, and I
also wasn't being strict enough when matching short circuited resource
paths (needed to look for trailing '/' too).
http://gwt-code-reviews.appspot.com/72801
--~--~-~--~~~---~--~~
John, this seems awfully complicated, and a lot of that complixity is in
support of big public API that as far as I can see is unused.
Is all of this really necessary for us to tell that 2.0.0-rc 2.0.0, or
whatever convention it was that we settled on?
I also don't think this should gate
1 - 100 of 1359 matches
Mail list logo
