[gwt-contrib] Re: Breaks dom.DOM dependency on user.UserAgent

2008-12-03 Thread t . broyer
I'd rather put it elsewhere, as it would just move the problem but not solve issue 2815. http://code.google.com/p/google-web-toolkit/issues/detail?id=2815 http://gwt-code-reviews.appspot.com/401 --~--~-~--~~~---~--~~

[gwt-contrib] Re: getAbsoluteLeft/Top() returns incorrect value in IE when zoomed in

2008-12-09 Thread t . broyer
How about using the screen.deviceXDPI and related properties? See: - http://msdn.microsoft.com/en-us/library/cc849094(VS.85).aspx#DetectViaJava - http://msdn.microsoft.com/en-us/library/ms537625(VS.85).aspx - http://msdn.microsoft.com/en-us/library/ms533721(VS.85).aspx Something like

[gwt-contrib] Re: IE8 support

2009-04-24 Thread t . broyer
My 2 c€nts. ...and this patch could also address issue 2815 http://code.google.com/p/google-web-toolkit/issues/detail?id=2815 (and do not forget issue 2938: http://code.google.com/p/google-web-toolkit/issues/detail?id=2938 ) http://gwt-code-reviews.appspot.com/29803/diff/1/9 File

[gwt-contrib] Re: split up long var lines

2009-05-20 Thread t . broyer
I'm also concerned that it only splits var statements; are we sure GWT won't generate too deep expressions on other kind of statements? http://gwt-code-reviews.appspot.com/33826/diff/1/6 File dev/core/src/com/google/gwt/dev/js/JsBreakUpLargeVarStatements.java (right):

[gwt-contrib] Re: Move GlassPanel from incubator to GWT trunk

2009-06-17 Thread t . broyer
http://gwt-code-reviews.appspot.com/39806/diff/1/8 File user/src/com/google/gwt/user/theme/standard/public/gwt/standard/standard.css (right): http://gwt-code-reviews.appspot.com/39806/diff/1/8#newcode485 Line 485: filter: alpha(opacity=50); Aren't you missing the new -ms-filter for IE8 (in IE=8

[gwt-contrib] Add Gears' Blob::getBytes

2009-07-05 Thread t . broyer
Reviewers: t.broyer, Description: This patch adds getBytes() method to Blob (Gears 0.5.21.0) Please review this at http://galgwt-reviews.appspot.com/41604 Affected files: gears/src/com/google/gwt/gears/client/blob/Blob.java Index: gears/src/com/google/gwt/gears/client/blob/Blob.java

[gwt-contrib] Re: Add Gears' BlobBuilder

2009-07-06 Thread t . broyer
http://galgwt-reviews.appspot.com/41603/diff/1/2 File gears/src/com/google/gwt/gears/client/blobbuilder/BlobBuilder.java (right): http://galgwt-reviews.appspot.com/41603/diff/1/2#newcode43 Line 43: public void append(byte[] bytes) { On 2009/07/06 13:26:56, zundel wrote: I think you could wrap

[gwt-contrib] Fix for issue 3973

2009-08-20 Thread t . broyer
Reviewers: jlabanca, Description: This is the simplest possible fix for issue 3973: ensure that hookEvents and unhookEvents are called even when not using a synthesized iframe (FormPanelImpl and FormPanelImplIE6 already handle the case where 'iframe' is null) A better fix IMO would be to also

[gwt-contrib] Fix for issue 3528

2009-08-26 Thread t . broyer
Reviewers: jgw, Description: This patch adds an assertion in RootPanel.detachOnWindowClose to check that the wrapped widget's element isn't an ancestor of an already wrapped-widget's element. Please review this at http://gwt-code-reviews.appspot.com/61813 Affected files:

[gwt-contrib] Fix for issue 3388

2009-08-26 Thread t . broyer
Reviewers: jlabanca, Description: This patch adds DELETE, HEAD and PUT methods to RequestBuilder and opens the RequestBuilder(String,String) ctor to public visibility. The restriction to GET and POST was only because of very old Safari versions (2.x or maybe even earlier). I've also removed a

[gwt-contrib] Re: Fix for issue 3388

2009-08-26 Thread t . broyer
http://gwt-code-reviews.appspot.com/61812/diff/1/3 File user/test/com/google/gwt/http/client/RequestBuilderTest.java (right): http://gwt-code-reviews.appspot.com/61812/diff/1/3#newcode32 Line 32: // XXX: W3C's XMLHttpRequest requires it be the empty string On 2009/08/26 15:59:28, jlabanca

[gwt-contrib] Re: Set cache headers properly in embedded Jetty for hosted mode

2009-09-22 Thread t . broyer
My 2 c€nts. Overall, it leads to reworking the patch as an: if (method is GET (or HEAD)) { if (url contains .nocache.) { set no-cache headers } else if (url contains .cache.) { set forever-cache headers } } // otherwise (non-GET/HEAD, or non-nocache/cache URL), just do

[gwt-contrib] Fix for issue 4067 (make UiBinderGenerator stable)

2009-10-09 Thread t . broyer
Reviewers: Ray Ryan, jgw, mmendez, Description: Instead of using a monotonically increasing number in UiBinderWriter.declareDomIdHolder(), makes the field name related to the field the element will be injected in (in most cases). For the case where there's a need to generate a field name

[gwt-contrib] Re: Refactored DisclosurePanel and better DisclosurePanelParser

2009-10-14 Thread t . broyer
My 2 c€nts http://gwt-code-reviews.appspot.com/78817/diff/15/1029 File user/src/com/google/gwt/user/client/ui/DisclosurePanel.java (right): http://gwt-code-reviews.appspot.com/78817/diff/15/1029#newcode62 Line 62: super(DOM.createAnchor()); Couldn't you use a FocusImpl instead, and therefore

[gwt-contrib] Re: Refactored DisclosurePanel and better DisclosurePanelParser

2009-10-14 Thread t . broyer
On 2009/10/14 15:43:38, jgw wrote: Broadly, I'm not entirely sure what we're trying to achieve by promoting DisclosurePanelHeader to a public top-level class. It's pretty complex, for a seemingly simple problem. If you were to replace it wholesale within an application, you will pretty much be

[gwt-contrib] Use native JSON when supported

2009-10-25 Thread t . broyer
Reviewers: Ray Ryan, bobv, Description: This patch adds: - JsonUtils.safeParse - JsonUtils.stringify - JsonUtils.isArray (equivalent of ECMAScript 5's Array.isArray) - deferred binding implementations using either native support or emulation (eval() for the parsing)

[gwt-contrib] Re: Fix for issue 3374 (URL.encodeComponent limited to query-string uses)

2009-10-27 Thread t . broyer
For those following progress on this, or finding this issue later on; and to not leave a comment unanswered: http://gwt-code-reviews.appspot.com/87806/diff/1/2 File user/src/com/google/gwt/http/client/URL.java (right): http://gwt-code-reviews.appspot.com/87806/diff/1/2#newcode170 Line 170: *

[gwt-contrib] Re: Fix for issue 4067 (make UiBinderGenerator stable)

2009-10-27 Thread t . broyer
On 2009/10/27 18:26:49, Ray Ryan wrote: I'm obliged for the patch, Tom, but the fix for the actual issue turns out to be: - private static int domId = 0; + private int domId = 0; Also, I'm pretty sure the WidgetPlaceholderInterpreter change will result in duplicate field names. WPI

[gwt-contrib] API change for c.g.g.http.client.URL

2009-10-27 Thread t . broyer
Reviewers: Ray Ryan, Description: As discussed in issue 3374, this patch deprecates encodeComponent and decodeComponent (including the ones added yesterday) and replaces them with encodePathSegment/encodeQueryString and decodePathSegment/decodeQueryString. If you compare it to pre-r6470, it

[gwt-contrib] Re: Squelch ie6 popup iframe hack on ie7

2009-11-09 Thread t . broyer
Sorry to jump in without having been invited but, given http://code.google.com/p/google-web-toolkit/issues/detail?id=805 (among others), shouldn't the iframe shim be extended to all browsers instead? (Closure-library seems to be doing just this; or actually, it moves the responsibility to the

[gwt-contrib] Re: Squelch ie6 popup iframe hack on ie7

2009-11-09 Thread t . broyer
On 2009/11/09 14:49:12, jgw wrote: On 2009/11/09 14:34:54, t.broyer wrote: Sorry to jump in without having been invited but, given http://code.google.com/p/google-web-toolkit/issues/detail?id=805 (among others), shouldn't the iframe shim be extended to all browsers instead?

[gwt-contrib] Various PopupPanel fixes

2009-11-26 Thread t . broyer
Reviewers: jgw, Ray Ryan, Description: Fixes issues 2907, 4277 and 4278. Issue 2907: use currentStyle instead of style to get the style of the popup (as suggested in the issue tracker) Issue 4277: that's a bit quick and dirty: I set the glass.className to the same as the

[gwt-contrib] Re: Various PopupPanel fixes

2009-11-26 Thread t . broyer
I finally managed to run RemoteWeb tests (PopupTest, DecoratedPopupTest and DialogBoxTest in -prod mode; on a Vista box with IE8 –which fortunately runs in compat mode and tests the user.agent=ie6 permutation–). It turned out there were a few bugs, and tests had to be updated (because I create the

[gwt-contrib] Re: Fix for issue 3528

2009-11-28 Thread t . broyer
On 2009/09/03 19:48:25, jgw wrote: On 2009/08/26 15:21:26, t.broyer wrote: Just so you know, I haven't forgotten about this, but I'm really swamped this week. I'm out next week, but will have a look at this as soon as I get back (unless someone else beats me to it). Just so you know ;-)

[gwt-contrib] Refactor SessionHandler and BrowserChannelClient to allow other OOPHM clients than HtmlUnit

2009-11-28 Thread t . broyer
Reviewers: amitmanjhi, Description: There's a TODO(amitmanjhi) in HtmlUnitSessionHandler to refactor SessionHandler, and the current situation makes it impossible for me to implement OOPHM in Adobe AIR (well, unless I write a full-fledged OOPHM client). So here's a patch that factors out a

[gwt-contrib] Re: Fix for issue 3388

2009-11-28 Thread t . broyer
Ping! will it make it into GWT 2.0? seems unlikely unfortunately, whereas the patch was LGTM three months ago :-( I'm submitting an updated patch, just in case, as the tests were changed due to FF3.5 supporting CORS (i.e. not throwing an exception in the cross-origin test)

[gwt-contrib] Adds manual user tests (-runStyle Manual) targets to user/build.xml

2009-11-28 Thread t . broyer
Reviewers: jat, Description: This patch adds -runStyle Manual test targets to user/build.xml; the number of clients is set in the ${gwt.hosts.web.manual} and ${gwt.hosts.dev.manual} properties. This is a follow-up to our discussion at

[gwt-contrib] Re: Adds manual user tests (-runStyle Manual) targets to user/build.xml

2009-11-28 Thread t . broyer
OK, ignore this one; I was patiently waiting for the Please navigate X browsers to this URL when I realized the stdout was redirected. So back to remote and selenium tests :-( http://gwt-code-reviews.appspot.com/114802 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Trivial fix for issue 3956 (REGRESSION: r5292 broke Opera support for History)

2009-12-18 Thread t . broyer
Reviewers: jgw, Description: This is a *really* trivial fix. Please review this at http://gwt-code-reviews.appspot.com/126812 Affected files: user/src/com/google/gwt/user/History.gwt.xml Index: user/src/com/google/gwt/user/History.gwt.xml

[gwt-contrib] Use cellIndex and sectionRowIndex in HTMLTable::getCellForEvent

2009-12-18 Thread t . broyer
Reviewers: jgw, Description: See http://groups.google.com/group/google-web-toolkit-contributors/t/a0b141cf7ea77a56 This is done by using com.google.gwt.dom.DOM's TableCellElement and TableRowElement, so I also changed everything (exception public/protected APIs) from

[gwt-contrib] Re: Benchmark for widget creation times

2009-12-22 Thread t . broyer
Wearing my nitpicker hat ;-) How about replacing the VerticalPanel and HorizontalPanel with FlowPanels too? (replace the HTML() label with InlineHTML() to make it show on the same line as the ListBox and Button) Or even use UiBinder to create the UI? (people might look at reference applications

[gwt-contrib] Re: API change for c.g.g.http.client.URL

2010-01-13 Thread t . broyer
I'll try to provided an updated patch asap (I have no devenv at hand right now) In the mean time, you can still comment on http://gwt-code-reviews.appspot.com/88802/patch/1/4 ;-) BTW, thanks for looking into it again! http://gwt-code-reviews.appspot.com/88802/diff/1/6 File

[gwt-contrib] Re: History methods NPE when iframe not included

2010-02-06 Thread t . broyer
http://gwt-code-reviews.appspot.com/138805/diff/1/6 File user/src/com/google/gwt/user/client/History.java (right): http://gwt-code-reviews.appspot.com/138805/diff/1/6#newcode64 Line 64: // Set impl to null as a flag to no-op future calls. This should test if 'impl' is null too; it's a different

[gwt-contrib] Re: Use cellIndex and sectionRowIndex in HTMLTable::getCellForEvent

2010-02-09 Thread t . broyer
Ping! ;-) http://gwt-code-reviews.appspot.com/125806 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Trivial fix for issue 3956 (REGRESSION: r5292 broke Opera support for History)

2010-02-09 Thread t . broyer
Ping! http://gwt-code-reviews.appspot.com/126812 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Refactor SessionHandler and BrowserChannelClient to allow other OOPHM clients than HtmlUnit

2010-02-09 Thread t . broyer
On 2009/12/10 14:49:27, jat wrote: Ok, I think we can work on getting this into trunk now. To do that, we need to do the cleanups you mention, such as getting rid of methods that only throw UOE from SessionHandler. Any chance of having it into 2.1?

[gwt-contrib] Fix for KeyPressEvent.getCharCode and overall key events improvements

2010-02-09 Thread t . broyer
Reviewers: jlabanca, Description: See http://groups.google.fr/group/Google-Web-Toolkit-Contributors/t/e705905e2cc78408 This patch: - fixes NativeEvent::getKeyCode so it always returns e.keyCode - introduces NativeEvent::getCharCode to return (e.keyCode || e.charCode) as suggested by PPK (this

[gwt-contrib] Re: Fix for KeyPressEvent.getCharCode and overall key events improvements

2010-02-10 Thread t . broyer
On 2010/02/10 17:20:07, jgw wrote: The first message on the linked thread suggests that the W3C has finally settled on a standard for keyboard events. The lack of such a standard is one reason this stuff is such a mess in the first place. Does anyone know where this is specified? Last

[gwt-contrib] Re: PopupPanel.setVisible does not affect glass

2010-02-12 Thread t . broyer
Your tests don't check that the iframe shim attached to the glass element in IE6/7 is hidden/shown. http://gwt-code-reviews.appspot.com/143809/diff/1/3 File user/src/com/google/gwt/user/client/ui/PopupPanel.java (right): http://gwt-code-reviews.appspot.com/143809/diff/1/3#newcode927 Line 927:

[gwt-contrib] Re: Fix for KeyPressEvent.getCharCode and overall key events improvements

2010-02-17 Thread t . broyer
I spent an hour or so re-reading http://unixpapa.com/js/key.html and it's quite clear that we actually need deferred-binding to gets things OK more easily, pretending that eventGetCharCode can only be called on a KeyPress event and eventGetKeyCode can only be called on KeyDown and KeyUp events

[gwt-contrib] Re: Make ServletValidator not use Jetty (issue284801)

2010-03-30 Thread t . broyer
Looks like it'd also fix issue 4462 http://code.google.com/p/google-web-toolkit/issues/detail?id=4462 (just to make sure it'll be marked fixed at the same time in the issue tracker) http://gwt-code-reviews.appspot.com/284801/diff/1/2 File dev/core/src/com/google/gwt/dev/ServletValidator.java

[gwt-contrib] Re: Add MD5 implementation, String byte manipulation methods/constructors. (issue516801)

2010-05-12 Thread t . broyer
http://gwt-code-reviews.appspot.com/516801/diff/1/4 File user/super/com/google/gwt/emul/java/lang/String.java (right): http://gwt-code-reviews.appspot.com/516801/diff/1/4#newcode458 user/super/com/google/gwt/emul/java/lang/String.java:458: int n = str.length(); There's actually a much easier

[gwt-contrib] Re: Add MD5 implementation, String byte manipulation methods/constructors. (issue516801)

2010-05-13 Thread t . broyer
http://gwt-code-reviews.appspot.com/516801/diff/1/4 File user/super/com/google/gwt/emul/java/lang/String.java (right): http://gwt-code-reviews.appspot.com/516801/diff/1/4#newcode458 user/super/com/google/gwt/emul/java/lang/String.java:458: int n = str.length(); On 2010/05/13 03:24:38, jat

[gwt-contrib] Fix issue 4781: allow HTML= in UiBinder for setHTML() (issue642801)

2010-06-18 Thread t . broyer
Reviewers: Ray Ryan, Description: Uses the java.beans.Introspector#decapitalize instead of manual decapitalization of setter-method names to attribute names. This means setHTML() can be set using an all-caps HTML= attribute, instead of hTML=. This applies to all all-caps setters (actually,

[gwt-contrib] Fix issue 150: regression re. keyCode/charCode (issue643801)

2010-06-18 Thread t . broyer
Reviewers: jlabanca, Description: As a result of r7692, NativeEvent#getKeyCode no longer returns the charCode for KeyPress events. This leads to better accuracy in handling keyboard-related events, but some widgets still use getKeyCode on KeyPress events, which in some (many? most?) browsers

[gwt-contrib] Re: Escaping HTML strings from the client as a good practice to avoid XSS vulnerabilities in apps th... (issue619803)

2010-06-22 Thread t . broyer
Sorry to step in without being invited (though I was earlier told that it should be expected when developing in the open), but... (see below) http://gwt-code-reviews.appspot.com/619803/diff/1/2 File user/src/com/google/gwt/user/tools/RpcServerTemplate.javasrc (right):

[gwt-contrib] Re: Fix external issue 5052 - JSONParser.parse exceptions with some unicode characters (issue659801)

2010-06-24 Thread t . broyer
Shouldn't the same be done in JsonUtils.unsafeEval? We could also at the same time remove the special handling of U+2028 and U+2029 from JsonUtils.escapeValue (though I really don't think this is worth it, as it still produces perfectly legal JSON; but maybe add a note to say that we're

[gwt-contrib] Re: Fix external issue 5052 - JSONParser.parse exceptions with some unicode characters (issue659801)

2010-06-25 Thread t . broyer
http://gwt-code-reviews.appspot.com/659801/diff/9001/10001 File user/src/com/google/gwt/core/client/JsonUtils.java (right): http://gwt-code-reviews.appspot.com/659801/diff/9001/10001#newcode76 user/src/com/google/gwt/core/client/JsonUtils.java:76: return typeof JSON == object typeof JSON.parse

[gwt-contrib] Re: Fix external issue 5052 - JSONParser.parse exceptions with some unicode characters (issue659801)

2010-06-25 Thread t . broyer
Conclusion: Safari 5 is half-baked :-( http://gwt-code-reviews.appspot.com/659801/diff/9001/10001 File user/src/com/google/gwt/core/client/JsonUtils.java (right): http://gwt-code-reviews.appspot.com/659801/diff/9001/10001#newcode76 user/src/com/google/gwt/core/client/JsonUtils.java:76: return

[gwt-contrib] Fix issue 5061: backslashes are not escaped by UiBinder in HasHTML and HasText (issue620804)

2010-06-25 Thread t . broyer
Reviewers: Ray Ryan, Description: UiBinderWriter#escapeTextForJavaStringLiteral correctly escapes \n and , but forgets \. Also, I couldn't see a reason for using replaceAll instead of replace. Please review this at http://gwt-code-reviews.appspot.com/620804/show Affected files:

[gwt-contrib] Fix issue 5065: Float.parseFloat and Double.parseDouble are too strict validating input (issue647802)

2010-06-28 Thread t . broyer
Reviewers: , Description: Make Float.parseFloat/valueOf and Double.parseDouble/valueOf accept strings with a float type suffix, such as 1.0f or 1.0d. Please review this at http://gwt-code-reviews.appspot.com/647802/show Affected files: user/super/com/google/gwt/emul/java/lang/Number.java

[gwt-contrib] Re: Fix external issue 5052 - JSONParser.parse exceptions with some unicode characters (issue659801)

2010-06-28 Thread t . broyer
http://gwt-code-reviews.appspot.com/659801/diff/9001/10001 File user/src/com/google/gwt/core/client/JsonUtils.java (right): http://gwt-code-reviews.appspot.com/659801/diff/9001/10001#newcode89 user/src/com/google/gwt/core/client/JsonUtils.java:89: out[0xad] = '\\u00ad'; // Soft hyphen On

[gwt-contrib] Re: Rolling back r8329 because it hands in IE8. (issue630802)

2010-06-29 Thread t . broyer
On 2010/06/29 21:56:58, jlabanca wrote: I think all of the doTestParseUnescaped loops in JSONTest are causing the tests to timeout. We'll have to break these tests down so they don't timeout. How about taking advantage of being in a GWTTestCase and use a Scheduler.RepeatingCommand?

[gwt-contrib] Re: Add WebSocket API (issue646803)

2010-07-01 Thread t . broyer
In a few words: everything's in c.g.g.dom, bringing a dependency from c.g.g.dom to c.g.g.event and breaking the assumption that an EventHandler is coupled with a GwtEvent (rather than a JSO-derivative), and it starts to refactor event handling at the low-level, while Joel is working on it (see

[gwt-contrib] Re: Fix issue 4781: allow HTML= in UiBinder for setHTML() (issue642801)

2010-07-02 Thread t . broyer
http://gwt-code-reviews.appspot.com/642801/diff/1/3 File user/test/com/google/gwt/uibinder/rebind/model/OwnerFieldClassTest.java (right): http://gwt-code-reviews.appspot.com/642801/diff/1/3#newcode62 user/test/com/google/gwt/uibinder/rebind/model/OwnerFieldClassTest.java:62:

[gwt-contrib] Re: Adds EventTarget, add/removeEventListener() et al to gwt dom. (issue623803)

2010-07-02 Thread t . broyer
LGTM overall. As a side note, where is the c.g.g.dom.client.EventListener defined? Could it be made generic? I like how the WebSocket proposal added a base Event class mimicking the DOM Event interface, with NativeEvent (and other WebSocket-specific events) extending it. EventListener could

[gwt-contrib] Re: Adds Java classes for JSON serialization/deserialization to complement (issue696801)

2010-07-15 Thread t . broyer
How is this better than existing JSON libraries, such as Google GSON? http://code.google.com/p/google-gson/ Moreover, GSON's use of reflection (JsonDeserializatonVisitor ObjectNavigator) might make it possible to use it for RequestFactory (both servlet, and client-side code generation) with

[gwt-contrib] Re: Puts PlaceController in the business of showing the user (issue698801)

2010-07-16 Thread t . broyer
LGTM (awesome! next step: integration with History ;-) ) http://gwt-code-reviews.appspot.com/698801/diff/8001/9002 File user/src/com/google/gwt/app/place/AbstractRecordEditActivity.java (right): http://gwt-code-reviews.appspot.com/698801/diff/8001/9002#newcode62

[gwt-contrib] Re: History integration for the RequestFactory apps. (issue717801)

2010-08-13 Thread t . broyer
I only quickly read through the list of added files and some of the files' content, but I can say I really like it the way it is (with PlaceTokenizer instead of the static fromToken method described in the Wave) There's a nit though, which should be probably fixed rather sooner than later ;-)

[gwt-contrib] Re: Added all safehtml packages. (issue771801)

2010-08-16 Thread t . broyer
I really like the interface/generator idea but not really the implementation (XML parsing –even though I could live with this–, generating XML –the HTML5 serialization algorithm [1] is fortunately easy enough and interoperable–) The HtmlSanitizer is a good idea, but the implementation is very

[gwt-contrib] Re: Continuation of r8542 to actually properly enables double click for (issue774801)

2010-08-17 Thread t . broyer
LGTM, though I'm puzzled by Hyperlink implementing HasDoubleClickHandlers, given that addClickHandler is deprecated on that class. http://gwt-code-reviews.appspot.com/774801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Added all safehtml packages. (issue771801)

2010-08-17 Thread t . broyer
On 2010/08/17 20:19:04, xtof wrote: On 2010/08/17 18:39:28, jat wrote: On 2010/08/16 23:39:47, tbroyer wrote: The HtmlSanitizer is a good idea, but the implementation is very weak [2]. Note that the API is what is important, and SimpleHtmlSanitizer is just that, a simple

[gwt-contrib] Re: Added all safehtml packages. (issue771801)

2010-08-18 Thread t . broyer
On 2010/08/17 23:23:39, xtof wrote: On 2010/08/17 23:05:06, tbroyer wrote: Looking at the code more closely it would merely fail by overly rejecting tags that are whitelisted: i.e. b foo=ishould be bold would be sanitized to lt;b foo=ishould be bold and the end part would be italicized

[gwt-contrib] Re: Added all safehtml packages. (issue771801)

2010-08-20 Thread t . broyer
http://gwt-code-reviews.appspot.com/771801/diff/11002/15007 File user/src/com/google/gwt/safehtml/shared/OnlyToBeUsedInGeneratedCodeStringBlessedAsSafeHtml.java (right): http://gwt-code-reviews.appspot.com/771801/diff/11002/15007#newcode38

[gwt-contrib] Re: Added all safehtml packages. (issue771801)

2010-08-23 Thread t . broyer
http://gwt-code-reviews.appspot.com/771801/diff/28002/6006 File user/src/com/google/gwt/safehtml/shared/OnlyToBeUsedInGeneratedCodeStringBlessedAsSafeHtml.java (right): http://gwt-code-reviews.appspot.com/771801/diff/28002/6006#newcode32

[gwt-contrib] Re: Fix the escaping done by UrlBuilder. (issue754803)

2010-08-24 Thread t . broyer
http://gwt-code-reviews.appspot.com/754803/diff/1/2 File user/src/com/google/gwt/http/client/UrlBuilder.java (right): http://gwt-code-reviews.appspot.com/754803/diff/1/2#newcode69 user/src/com/google/gwt/http/client/UrlBuilder.java:69: for (String segment : path.split(/)) { It'd probably be

[gwt-contrib] Allow a fallback/catch-all and prefix-less PlaceTokenizer makes Historian pluggable (issue824801)

2010-09-01 Thread t . broyer
Reviewers: rjrjr, Description: Fixes https://jira.springsource.org/browse/ROO-1276 First, the anonymous-class implementation of Historian is moved into a public static DefaultHistorian nested class that's GWT.create()d to allow pluggability via a replace-with rule in a gwt.xml file (similar to

[gwt-contrib] Addresses various issues with PlaceHistoryHandlerGenerator (issue827801)

2010-09-01 Thread t . broyer
Reviewers: , Description: This is a major refactor of PlaceHistoryHandlerGenerator/PlaceHistoryGeneratorContext to address various issues with the current implementation: - place types should be ordered from most-derived to least-derived in the generated getPrefixAndToken (see

[gwt-contrib] Re: Fix the escaping done by UrlBuilder. (issue754803)

2010-09-02 Thread t . broyer
I don't quite understand what this change in RemoteServiceServlet has to do with UrlBuilder... http://gwt-code-reviews.appspot.com/754803/diff/21001/22002 File user/src/com/google/gwt/user/server/rpc/RemoteServiceServlet.java (right):

[gwt-contrib] Re: Fix the escaping done by UrlBuilder. (issue754803)

2010-09-02 Thread t . broyer
In brief: URIs (RFC 3986) is a mess, and back to URL.encode for the path in UrlBuilder? (with a second pass though) http://gwt-code-reviews.appspot.com/754803/diff/21001/22002 File user/src/com/google/gwt/user/server/rpc/RemoteServiceServlet.java (right):

[gwt-contrib] Alternate PlaceHistoryHandler design/API (issue845802)

2010-09-05 Thread t . broyer
Reviewers: rjrjr, Description: Following up on https://jira.springsource.org/browse/ROO-1276, here's the proposed alternate design, separating the keep history and places in sync and map places to/from history tokens concerns into a concrete PlaceHistoryHandler class and a PlaceHistoryMapper

[gwt-contrib] Adds getInheritableMethods to JClassType (issue863802)

2010-09-11 Thread t . broyer
Reviewers: bruce, scottb, Description: Similar to getOverridableMethods but includes final methods as well. This would make it easier to fix ROO-1297 and in general work with factories (such as the code generator for ActivityMapper I proposed in the Activity design Wave)

[gwt-contrib] Re: Adds getInheritableMethods to JClassType (issue863802)

2010-09-11 Thread t . broyer
I think I've spotted the reason for the test failure: it's actually a bug in the current getOverridableMethods, and a bug in JEnumTypeTest (it should have really expected 4 methods instead of 7). Reported as issue 5270: http://code.google.com/p/google-web-toolkit/issues/detail?id=5270

[gwt-contrib] Fixes issue 5270 (issue839803)

2010-09-11 Thread t . broyer
Reviewers: bruce, scottb, Description: http://code.google.com/p/google-web-toolkit/issues/detail?id=5270 Description copied from the issue: In c.g.g.core.ext.typeinfo.AbstractMembers, final methods are simply ignored instead of being treated as possible overrides of already seen methods. A

[gwt-contrib] Re: Makes IsWidget a first class concept, and makes it convenient to (issue864801)

2010-09-12 Thread t . broyer
Are all these ForIsWidget interfaces (and method overloads) really needed? (and actually useful?) For completeness, TabLayoutPanel and TabPanel don't have IsWidget overloads for their add, getTabWidget and selectTab methods. (if you ask me, I'd only keep HasOneWidget, without the getWidget

[gwt-contrib] Re: Makes IsWidget a first class concept, and makes it convenient to (issue864801)

2010-09-13 Thread t . broyer
LGTM (FWIW, and speaking only about the API) http://gwt-code-reviews.appspot.com/864801/diff/3001/4034 File user/test/com/google/gwt/user/client/ui/SimplePanelTest.java (right): http://gwt-code-reviews.appspot.com/864801/diff/3001/4034#newcode35

[gwt-contrib] Re: Replaces the public and obnoxious String EntityProxy#getId() with the (issue902801)

2010-09-22 Thread t . broyer
http://gwt-code-reviews.appspot.com/902801/diff/20001/21013 File user/src/com/google/gwt/app/place/AbstractProxyEditActivity.java (right): http://gwt-code-reviews.appspot.com/902801/diff/20001/21013#newcode147 user/src/com/google/gwt/app/place/AbstractProxyEditActivity.java:147: stableId =

[gwt-contrib] Re: Fixing a bug with images used in CellTree, CellBrowser, and IconCellDecorator in browsers that b... (issue912801)

2010-09-23 Thread t . broyer
Looks like there's a regression in IconCellDecorator (re. valign) http://gwt-code-reviews.appspot.com/912801/diff/1/2 File user/src/com/google/gwt/cell/client/IconCellDecorator.java (right): http://gwt-code-reviews.appspot.com/912801/diff/1/2#newcode53

[gwt-contrib] Let ToggleButton implement HasValueBoolean (issue887804)

2010-09-23 Thread t . broyer
Reviewers: jgw, Description: also fixes issue 5076 (add ToggleButton(upText,downText,handler) constructor) and a few nits in the CheckBox javadoc (referring to text box, and bad doc re. null value handling in setValue) Assigning to Joel as per

[gwt-contrib] Re: Changes required to make the Scaffold app look like the mocks. Added null checks AbstractProxyLi... (issue925801)

2010-09-27 Thread t . broyer
http://gwt-code-reviews.appspot.com/925801/diff/1/2 File user/src/com/google/gwt/app/place/AbstractProxyListActivity.java (right): http://gwt-code-reviews.appspot.com/925801/diff/1/2#newcode256 user/src/com/google/gwt/app/place/AbstractProxyListActivity.java:256: if (view == null) { Isn't the

[gwt-contrib] Re: Changes required to make the Scaffold app look like the mocks. Added null checks AbstractProxyLi... (issue925801)

2010-09-28 Thread t . broyer
On 2010/09/27 23:49:02, rjrjr wrote: Point taken on the view contention front. I think Thomas is right that setting and clearing the view delegate should happen in start() and stop(), not in the constructor. Thanks, Thomas. You're welcome ;-) Sounds like the javadoc should say so. +1

[gwt-contrib] Re: Changes in com.google.gwt.app land to catch up to the RequestFactory (issue946802)

2010-10-04 Thread t . broyer
http://gwt-code-reviews.appspot.com/946802/diff/8001/9005 File user/src/com/google/gwt/app/place/CreateAndEditProxy.java (right): http://gwt-code-reviews.appspot.com/946802/diff/8001/9005#newcode54 user/src/com/google/gwt/app/place/CreateAndEditProxy.java:54: display.setWidget(null); Er, really

[gwt-contrib] Re: Let ToggleButton implement HasValueBoolean (issue887804)

2010-10-08 Thread t . broyer
On 2010/10/08 21:55:30, jgw wrote: On 2010/09/23 14:32:37, tbroyer wrote: LGTM. Committed at r8982. Thanks! http://gwt-code-reviews.appspot.com/887804/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Make sure redundant calls to an Activity's display setWidget are OK (issue999802)

2010-10-16 Thread t . broyer
Reviewers: rjrjr, Description: See one use-case in point 3 of http://groups.google.fr/group/google-web-toolkit-contributors/msg/af8578d95692aff5 Another use case: edit and details of a proxy is based on the proxy itself (it has an isReadOnly() property, and if it isn't read-only it's always in

[gwt-contrib] Re: Making Composite implement RequiresResize and ProvidesResize so that it can pass resize informat... (issue1041801)

2010-10-21 Thread t . broyer
Isn't that duplicating the functionnality of the ResizeComposite? http://google-web-toolkit.googlecode.com/svn/javadoc/2.1/com/google/gwt/user/client/ui/ResizeComposite.html Should it then be deprecated? At least it should itself implement ResizeComposite, as it would now inherit the behavior

[gwt-contrib] Re: Overhaul the RequestFactory server code. (issue1062801)

2010-10-29 Thread t . broyer
http://gwt-code-reviews.appspot.com/1062801/diff/1/75 File user/src/com/google/gwt/requestfactory/server/ReflectiveServiceLayer.java (right): http://gwt-code-reviews.appspot.com/1062801/diff/1/75#newcode77 user/src/com/google/gwt/requestfactory/server/ReflectiveServiceLayer.java:77: return

[gwt-contrib] Re: Overhaul the RequestFactory server code. (issue1062801)

2010-10-29 Thread t . broyer
http://gwt-code-reviews.appspot.com/1062801/diff/1/78 File user/src/com/google/gwt/requestfactory/server/RequestFactoryInterfaceValidator.java (right): http://gwt-code-reviews.appspot.com/1062801/diff/1/78#newcode947

[gwt-contrib] Re: Loosens up JSO restrictions in the TypeOracle to allow multiple JSO (issue1076801)

2010-11-03 Thread t . broyer
http://gwt-code-reviews.appspot.com/1076801/diff/1/2 File dev/core/src/com/google/gwt/core/ext/typeinfo/TypeOracle.java (right): http://gwt-code-reviews.appspot.com/1076801/diff/1/2#newcode676 dev/core/src/com/google/gwt/core/ext/typeinfo/TypeOracle.java:676: // TODO(jgw): Not sure if

[gwt-contrib] Re: First-pass for adding HTML5's Canvas. (issue1082801)

2010-11-05 Thread t . broyer
http://gwt-code-reviews.appspot.com/1082801/diff/1/3 File user/src/com/google/gwt/html5/canvas/client/Canvas.java (right): http://gwt-code-reviews.appspot.com/1082801/diff/1/3#newcode52 user/src/com/google/gwt/html5/canvas/client/Canvas.java:52: * @return the height, in pixels Are they really

[gwt-contrib] Re: First-pass for adding HTML5's Canvas. (issue1082801)

2010-11-07 Thread t . broyer
http://gwt-code-reviews.appspot.com/1082801/diff/1/10 File user/src/com/google/gwt/html5/canvas/client/CssColor.java (right): http://gwt-code-reviews.appspot.com/1082801/diff/1/10#newcode23 user/src/com/google/gwt/html5/canvas/client/CssColor.java:23: public class CssColor implements

[gwt-contrib] Re: First-pass for adding HTML5's Canvas. (issue1082801)

2010-11-07 Thread t . broyer
http://gwt-code-reviews.appspot.com/1082801/diff/1/7 File user/src/com/google/gwt/html5/canvas/client/CanvasPixelArray.java (right): http://gwt-code-reviews.appspot.com/1082801/diff/1/7#newcode41 user/src/com/google/gwt/html5/canvas/client/CanvasPixelArray.java:41: return this[i]; On 2010/11/07

[gwt-contrib] Re: First-pass for adding HTML5's Canvas. (issue1082801)

2010-11-10 Thread t . broyer
http://gwt-code-reviews.appspot.com/1082801/diff/1/4 File user/src/com/google/gwt/html5/canvas/client/CanvasElement.java (right): http://gwt-code-reviews.appspot.com/1082801/diff/1/4#newcode42 user/src/com/google/gwt/html5/canvas/client/CanvasElement.java:42: public final native Context

[gwt-contrib] Issues 5479, 5507 and 5571 (and a few other editor-related tweaks) (issue1099801)

2010-11-11 Thread t . broyer
Reviewers: bobv, Description: This patch: - reuses editors in every IsEditor widget (issue 5479) - adds a ValueLabel widget, and DateLabel and NumberLabel subclasses (issue 5507), introduces a NumberFormatRenderer (similar to the existing DateTimeFormatRenderer) for use in NumberLabel (this

[gwt-contrib] Re: Issues 5479, 5507 and 5571 (and a few other editor-related tweaks) (issue1099801)

2010-11-11 Thread t . broyer
Thanks for the quick feedback! http://gwt-code-reviews.appspot.com/1099801/diff/1/8 File user/src/com/google/gwt/user/client/ui/NumberLabel.java (right): http://gwt-code-reviews.appspot.com/1099801/diff/1/8#newcode22 user/src/com/google/gwt/user/client/ui/NumberLabel.java:22: * A ValueLabel

[gwt-contrib] Re: Issues 5479, 5507 and 5571 (and a few other editor-related tweaks) (issue1099801)

2010-11-13 Thread t . broyer
http://gwt-code-reviews.appspot.com/1099801/diff/1/8 File user/src/com/google/gwt/user/client/ui/NumberLabel.java (right): http://gwt-code-reviews.appspot.com/1099801/diff/1/8#newcode34 user/src/com/google/gwt/user/client/ui/NumberLabel.java:34: } On 2010/11/11 19:06:31, jat wrote: On

[gwt-contrib] Re: Issues 5479, 5507 and 5571 (and a few other editor-related tweaks) (issue1099801)

2010-11-15 Thread t . broyer
The 3rd patch set adds tests, UiBinder element parsers for DateLabel and NumberLabel, and the ToggleButton (issue 5571) which I forgot in previous patches. I found some issues with generics in UiBinder and had to relax a check in UiBinderParser (but I believe this is taken care of later on, see

[gwt-contrib] Re: Issues 5479, 5507 and 5571 (and a few other editor-related tweaks) (issue1099801)

2010-11-18 Thread t . broyer
http://gwt-code-reviews.appspot.com/1099801/diff/26001/27004 File user/src/com/google/gwt/uibinder/elementparsers/DateLabelParser.java (right): http://gwt-code-reviews.appspot.com/1099801/diff/26001/27004#newcode36 user/src/com/google/gwt/uibinder/elementparsers/DateLabelParser.java:36: static

[gwt-contrib] Re: Issues 5479, 5507 and 5571 (and a few other editor-related tweaks) (issue1099801)

2010-11-19 Thread t . broyer
http://gwt-code-reviews.appspot.com/1099801/diff/26001/27009 File user/src/com/google/gwt/user/client/ui/DateLabel.java (right): http://gwt-code-reviews.appspot.com/1099801/diff/26001/27009#newcode25 user/src/com/google/gwt/user/client/ui/DateLabel.java:25: * A ValueLabel that uses {...@link

[gwt-contrib] Re: Add Locator API to allow arbitrary domain types to be used with RequestFactory. (issue1130801)

2010-11-19 Thread t . broyer
http://gwt-code-reviews.appspot.com/1130801/diff/1/10 File user/src/com/google/gwt/requestfactory/server/LocatorServiceLayer.java (right): http://gwt-code-reviews.appspot.com/1130801/diff/1/10#newcode146 user/src/com/google/gwt/requestfactory/server/LocatorServiceLayer.java:146: Locator?, ?

[gwt-contrib] Issue 5474: RequestFactory response encoding (issue1154801)

2010-11-27 Thread t . broyer
Reviewers: bobv, Description: Fix for RequestFactoryServlet and unit test. Also fixed testUserInfo in RequestFactoryTest, which was not asynchronous (and therefore, I believe, wasn't actually testing anything). The tests use finishTest and not finishTestAndReset because they don't use

  1   2   3   4   5   >