[gwt-contrib] Fwd: Intent to Deprecate and Freeze: The User-Agent string

[t . broyer]

This is going to directly impact the selection script and permutation-based 
compilation of GWT 2.x.
I've read several times that some people use a single user.agent value 
(can't remember if it's gecko1_8 or safari), do we have some guarantees 
that this will work reliably across "evergreen browsers"? Time to make 
gecko1_8 and safari permutations converge? (and let's declare ie8/ie9/ie10 
as dead; in other words, time to remove UA-based permutations even before 
GWT3/J2Cl?)

On Tuesday, January 14, 2020 at 12:22:00 PM UTC+1, Yoav Weiss wrote:
>
> Contact emails
>
> yoavwe...@chromium.org , aaron...@chromium.org 
>
> Summary
>
> We want to freeze and unify (but not remove) the User Agent string in 
> HTTP requests as well as in `navigator.userAgent`
>
> Motivation
>
> The User-Agent string is an abundant source of passive fingerprinting 
> information about our users. It contains many details about the user’s 
> browser and device as well as many lies ("Mozilla/5.0", anyone?) that were 
> or are needed for compatibility purposes, as servers grew reliant on bad 
> User Agent sniffing.
>
> On top of those privacy issues, User-Agent sniffing is an abundant source 
> of compatibility issues, in particular for minority browsers, resulting in 
> browsers lying about themselves (generally 
> 
>  
> or to specific sites 
> ),
>  
> and sites (including Google properties) being broken 
> 
>  
> in some browsers for no good reason.
>
> The above abuse makes it desirable to freeze the UA string and replace it 
> with a better mechanism. There have been past attempts at UA string 
> freezing from the Safari team, but without an alternative way to perform UA 
> based content-negotiation, they had to be partially reverted.
>
>  
>
> The User Agent Client Hints  
> (UA-CH) feature provides an alternate source for the information the 
> User-Agent string provides, both in its request header form as well as its 
> JS API one. 
>
> Its main advantages are:
>
>- 
>
>It provides the required information only when the server requests it, 
>over secure connections, making any fingerprinting that relies on it be 
> active 
>fingerprinting, which enables such use to be audited, as well as 
>acted-upon by the browser (e.g. in a future implementation of the Privacy 
>Budget ).
>- 
>
>It provides the information in small increments, so servers are only 
>exposed to the information they need and request, rather than being 
> exposed 
>to the full gamut of the UA string even if they are just trying to figure 
>out one detail about the browser. (e.g. brand and major version)
>- 
>
>Since it provides the information via dedicated fields, it enables 
>better ergonomics and makes it less likely for servers to get it wrong and 
>cause compatibility issues.
>- 
>
>And finally, starting fresh will enable us to drop a lot of the legacy 
>baggage that the UA string carries (“Mozilla/5.0”, “like Gecko”, “like 
>KHTML”, etc) going forward.
>
>
> Once UA-CH ships 
> 
>  
> as an alternative means for browser-specific content adaptation, we would 
> like to freeze the User-Agent string. 
>
> We propose to deprecate at M81 (starting to emit console warnings in pages 
> that read that string in JS), freeze its version information at M83, and 
> unify strings of different devices at M85. See detailed freezing plan 
> below. 
>
> This timeline provides 3 months for developers to move to the new 
> mechanism for their future browser and OS version needs, and 6 months for 
> more sophisticated OS or device specific targeting.
>
> Freezing plan
>
> Different parts of the UA string have different compatibility 
> implications. 
>
> Some parts of it, such as the browser version and the OS version, can be 
> frozen without any backwards compatibility implications. Values that worked 
> in the past will continue to work in the future.
>
> Other parts, such as the model (for mobile devices) and the OS platform, 
> can have implications on sites that tailor their UI to the underlying OS or 
> that target a very specific model in their layout. Such sites will need to 
> migrate to use UA-CH.
>
>
> As such we are planning to freeze the parts that are amenable to freezing 
> fairly early, and gradually unify the rest.
>
>
> Milestone
>
> Stable date
>
> Action
>
> M81
>
> Mid March ‘20
>
> Deprecate access to `navigator.userAgent` 
>
> M83
>
> Early June ‘20
>
> 

Re: Security Vulnerability Detected in GWT Library

[t . broyer]

On Wednesday, May 1, 2019 at 8:58:03 PM UTC+2, foal wrote:
>
> Easly to update in upcoming releases than explain each other that it isn't 
> critical :)
>
> BTW GWT-RPC user protobuf?
>

The protobuf in gwt-servlet is an internal dependency for sourcemaps and 
streamhtmlparser (used in server-side SafeHtml)
See https://github.com/gwtproject/gwt/issues/9659
 

> Thought about replacing REST with Protobuf but did not find ready to use 
> solution (Java <-> GWT with APT generators).
>

Maybe grpc-web would be usable nowadays? 

-- 
You received this message because you are subscribed to the Google Groups "GWT 
Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit+unsubscr...@googlegroups.com.
To post to this group, send email to google-web-toolkit@googlegroups.com.
Visit this group at https://groups.google.com/group/google-web-toolkit.
For more options, visit https://groups.google.com/d/optout.

Re: Keyboard navigation of modal DialogBox escaping the dialog

[t . broyer]

On Wednesday, May 1, 2019 at 8:39:37 PM UTC+2, ga...@rstudio.com wrote:
>
> Existing application using Gwt 2.8.2, and I'm undertaking effort to make 
> it accessible (keyboard, screen reader, and so forth). Always a challenge 
> to retrofit accessibility when it wasn't considered from the start, but 
> that's my lot in life (cue violins).
>
> Problem of the day, modal DialogBoxes don't keep focus in the dialogs when 
> using tab key. Several very old conversations on this, wondering if there's 
> anything more current, or if it's still something I'll need to handle 
> directly on a dialog-by-dialog basis. We have a lot of dialogs.
>
> Specifically, keyboard tabbing off final control should wrap around to 
> beginning of dialog, NOT to the browser itself or the glassed-out UI of the 
> application. Shift+Tabbing back from the first control should go to the 
> last. Fundamental accessibility requirement. For example, try any modal 
> dialog in Google Docs.
>

Modal popups in GWT have been broken for years (I asked that they be 
deprecated 8 YEARS ago: https://github.com/gwtproject/gwt/issues/6454), 
don't use them.

I you really want/need a modal dialog, I'd suggest using a  element 
with showModal(). The API is already available in 
Elemental: 
https://static.javadoc.io/com.google.elemental2/elemental2-dom/1.0.0-RC1/elemental2/dom/HTMLDialogElement.html#showModal--
And for browser compatibility (rather poor at the moment: 
https://caniuse.com/#feat=dialog), use the polyfill: 
https://github.com/GoogleChrome/dialog-polyfill (with GWT, you'd have to 
inject the script –e.g. using ScriptInjector– then use JsInterop to call 
the polyfill's registerDialog())

Alternatively, use a non-modal GWT dialog and implement the "modal" 
behavior "by hand": for tabbing, put hidden focusable elements at the 
"beginning" and "end" of the dialog, catch the focus event on those and 
then refocus to the appropriate element.

-- 
You received this message because you are subscribed to the Google Groups "GWT 
Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit+unsubscr...@googlegroups.com.
To post to this group, send email to google-web-toolkit@googlegroups.com.
Visit this group at https://groups.google.com/group/google-web-toolkit.
For more options, visit https://groups.google.com/d/optout.

[gwt-contrib] Time to require JDK 8?

[t . broyer]

Hi all,

3½ years ago, we announced 2.8.0-beta1 and that it now required JDK 7, and 
that started quite a long discussion: 
https://groups.google.com/d/topic/google-web-toolkit-contributors/TzsINiDf5xg/discussion
A few days ago, there's been renewed interest into upgrading the Jetty 
version GWT is using (for reasons I don't support, but that's orthogonal to 
the point): https://github.com/gwtproject/gwt/issues/9606
Upgrading Jetty however means requiring JDK 8; and *we have people 
volunteering to do this work*.

We're in April 2019, Oracle just gave the keys of OpenJDK 11 updates to 
RedHat, them already being the stewards for OpenJDK 8 and 7.
JDK 6 is dead and buried (Oracle's extended support ended last December, 
and it looks like even Azul Systems –the company that, to my knowledge, 
sells the longest support– no longer provides paid support for OpenJDK 6 
either)
OpenJDK 7 will receive free updates for only one more year (June 2020), 
though some vendors will provide paid support 'til 2022 (e.g. Azul, even 
providing "passive" support –whatever it means– 'til 2024); for Oracle JDK 
7, Oracle's Premier Support will end in July this year (tick tock tick 
tock), and extended support will last 'til 2022.
OpenJDK 8 will receive free updates until June 2023 (4 years from now, 
almost the same time that has passed since 2.8.0-beta1, just sayin')
For people sharing code with Android, correct me if I'm wrong, but latest 
tooling improvements (D8 
) mean that you can 
use libraries targeting JDK 8 even on older Android devices (basically, D8 
integrates retrolambda into the Android toolchain), so *even Android is no 
longer an excuse.*

*Maybe it's time to switch to JDK 8 as the new baseline for GWT*: by the 
time we release a 2.9 (who knows when), OpenJDK 7 will only have months to 
go (tick tock, 14 months from now).

Fwiw, my reasoning for mostly/only caring about free updates to the JDK are:

   - AFAICT, GWT receives no money. Some companies (possibly?) dedicate 
   time for maintaining GWT, but it mostly runs on the free time and free will 
   of a few people (I wouldn't even count myself in any longer)
   - Companies that run those older JDK versions likely pay for support. If 
   they have money for that, they can also build their own version of GWT that 
   still supports those older JDK versions (either adapting the most recent 
   GWT version to bring compatibility with older JDKs, or backport fixes to 
   older GWT versions).
   - Companies that run older JDKs without paying for support, besides 
   being crazy (particularly if their servers are exposed on the Internet), 
   can IMO live with older GWT versions too (for JDK 7, in a year from now, 
   that'll still include GWT up to 2.8.2, the current latest release)

-- 
You received this message because you are subscribed to the Google Groups "GWT 
Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/google-web-toolkit-contributors/079d422f-0294-43b4-94be-3915bb349fda%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

GWT RPC Multiplexing with Atmosphere

[Abraham T]

In my app GWT has a single RPC, I want to know if Atmosphere Async-IO can 
help  Multiplexing.
if possible, any resource would be great ?

Thank you!

-- 
You received this message because you are subscribed to the Google Groups 
Google Web Toolkit group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit+unsubscr...@googlegroups.com.
To post to this group, send email to google-web-toolkit@googlegroups.com.
Visit this group at http://groups.google.com/group/google-web-toolkit.
For more options, visit https://groups.google.com/d/optout.

Re: GWT 508 Compliance

[Balaji T]

Which version of GWT supports fully 508 compliance

On Friday, 3 September 2010 05:49:39 UTC+5:30, Google Integrator wrote:

 How would I make my GWT application 508 Compliant?  What are the best 
 strategies currently out there for this.  I am writing an application 
 that makes a lot of calls to get data from various servers and after 
 the data is returned, I would like a screen reader to read what is 
 returned in a readable way.  Any ideas, suggestions would be greatly 
 appreciated!

-- 
You received this message because you are subscribed to the Google Groups 
Google Web Toolkit group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit+unsubscr...@googlegroups.com.
To post to this group, send email to google-web-toolkit@googlegroups.com.
Visit this group at http://groups.google.com/group/google-web-toolkit.
For more options, visit https://groups.google.com/d/optout.

Which version of GWT fully supports 508 compliance

[Balaji T]

Hi,
I am working on a web project where in the UI part is developed using 
GWT 2.3 version, but recently we were asked to check for 508 compliance and 
we are not sure which version of GWT is 508 complaint.
Could any one help out on this.

-- 
You received this message because you are subscribed to the Google Groups 
Google Web Toolkit group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit+unsubscr...@googlegroups.com.
To post to this group, send email to google-web-toolkit@googlegroups.com.
Visit this group at http://groups.google.com/group/google-web-toolkit.
For more options, visit https://groups.google.com/d/optout.

Re: Export CellTable's selected rows into a csv file

[Winnie T]

Oh I see. I did something like this (below), wonder if you can spot the 
error when I handle the results that is being passed to the library

try {
session.beginTransaction( );   
// I am using hibernate session
Criteria criteria = session.createCriteria(Book.class); 
  // Create criteria to filter base on the type of 
search
criteria.add(Restrictions.like(field,% + search + 
%).ignoreCase());

   bookList = criteria.list( ); 
   //Call the search results to a list
String[] bookArray = new String[bookList.size()];   
 // Dump the result list into an string array (as 
CSVWriter could only take in string array, not list)
CSVWriter writer = new CSVWriter(new FileWriter(export.csv), 
'\t');   // Using OpenCSV library to write a new file export.csv
writer.writeNext(bookArray);   
  // Writes the string array line by line into 
export.csv   
writer.close();

session.getTransaction().commit();
return bookList;// return 
the search results to a cell table for displaying 

} catch (HibernateException e) {
log(Failed to retrie ve profile information from the 
database, e);
throw new RuntimeException(Failed to retrieve profile 
information from the database);
} catch (IOException e) {
e.printStackTrace();  //To change body of catch statement use 
File | Settings | File Templates.
}
return null;   //requires a return which I am not sure why
}

Thank you!! =)

Winnie

On Monday, August 12, 2013 7:49:39 PM UTC+8, Thad Humphries wrote:

 No, I was just saying how I did it. As I recall--it's been several 
 years--I first used OpenCSV but switched to Apache's CSV because it also 
 had a tab delimited option (though I guess you can do the same thing with 
 OpenCSV by changing the delimiters). 

 If your file is empty, I'd check your file calls, not the CSV library: Is 
 the file opened for writing, is the file handle properly passed to the 
 library, is the data still there to write, is the output properly flushed, 
 and is the file closed. I'd write something to the file before and after my 
 CVS calls to see if anything gets written. And I'd check the file on the 
 server, not just what my servlet returns.

 On Sunday, August 11, 2013 10:32:23 PM UTC-4, Winnie T wrote:

 Hi Thad, thanks for responding. Currently, I tried using OpenCSV, and had 
 successfully create a CSV file when I click the export button, but the CSV 
 file was empty. Do you meant that I could use Apache Commons instead of 
 OpenCSV?

 On Thursday, August 8, 2013 10:27:06 PM UTC+8, Thad Humphries wrote:

 If your server side still has a copy of your list, you can send 
 references to the rows by row number (i thru n) or some sort of row id set 
 in a servlet call. The servlet could build the CSV (Apache Commons has a 
 CSV module) and return it to the browser with the MIME text/csv so the 
 browser could decide what to do with it. You'll have to target your servlet 
 at a _blank page or an IFRAME to prevent stomping on your GWT app.

 On Thursday, February 3, 2011 2:42:26 AM UTC-5, Ido wrote:

 Well, the server side which should populate the csv file is the server 
 package within the GWT project.
 I'm not using any external backend to populate the csv file.
 When mentioned sending list of objects using RPC I meant using the 
 RemoteServiceServlet.

 Once my server get's this list , it should create a csv out of it and 
 *somehow* send it to the client.
 Just can't figure how to do so.
 Maybe the server should return the user a path to the created file, but 
 then what?
 Maybe new servlet should be created to handle this kind of action, but 
 how can I send the list of the selected objects?







-- 
You received this message because you are subscribed to the Google Groups 
Google Web Toolkit group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit+unsubscr...@googlegroups.com.
To post to this group, send email to google-web-toolkit@googlegroups.com.
Visit this group at http://groups.google.com/group/google-web-toolkit.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Export CellTable's selected rows into a csv file

[Winnie T]

Hi Thad, thanks for responding. Currently, I tried using OpenCSV, and had 
successfully create a CSV file when I click the export button, but the CSV 
file was empty. Do you meant that I could use Apache Commons instead of 
OpenCSV?

On Thursday, August 8, 2013 10:27:06 PM UTC+8, Thad Humphries wrote:

 If your server side still has a copy of your list, you can send references 
 to the rows by row number (i thru n) or some sort of row id set in a 
 servlet call. The servlet could build the CSV (Apache Commons has a CSV 
 module) and return it to the browser with the MIME text/csv so the browser 
 could decide what to do with it. You'll have to target your servlet at a 
 _blank page or an IFRAME to prevent stomping on your GWT app.

 On Thursday, February 3, 2011 2:42:26 AM UTC-5, Ido wrote:

 Well, the server side which should populate the csv file is the server 
 package within the GWT project.
 I'm not using any external backend to populate the csv file.
 When mentioned sending list of objects using RPC I meant using the 
 RemoteServiceServlet.

 Once my server get's this list , it should create a csv out of it and 
 *somehow* send it to the client.
 Just can't figure how to do so.
 Maybe the server should return the user a path to the created file, but 
 then what?
 Maybe new servlet should be created to handle this kind of action, but 
 how can I send the list of the selected objects?







-- 
You received this message because you are subscribed to the Google Groups 
Google Web Toolkit group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit+unsubscr...@googlegroups.com.
To post to this group, send email to google-web-toolkit@googlegroups.com.
Visit this group at http://groups.google.com/group/google-web-toolkit.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Export CellTable's selected rows into a csv file

[Winnie T]

Hi Ido,

could you explain how your successfully generate the search results in step 
3. I had done step 1 and 2 . In step 3, I managed to generate a csv file, 
but it is empty. Thank you.

WinnieT

-- 
You received this message because you are subscribed to the Google Groups 
Google Web Toolkit group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit+unsubscr...@googlegroups.com.
To post to this group, send email to google-web-toolkit@googlegroups.com.
Visit this group at http://groups.google.com/group/google-web-toolkit.
For more options, visit https://groups.google.com/groups/opt_out.

[gwt-contrib] Re: Fix for issue 5926 (at last!) (issue1594804)

[t . broyer]

Moved to https://gwt-review.googlesource.com/3831

http://gwt-code-reviews.appspot.com/1594804/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[gwt-contrib] Re: Use locale-insensitive checks in RF unit tests so they don't fail in non-English locales. (issue1430801)

[t . broyer]

Moved to https://gwt-review.googlesource.com/3832

http://gwt-code-reviews.appspot.com/1430801/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[gwt-contrib] Re: Remove long-deprecated event listeners and EventPreview. (issue1822803)

[t . broyer]

Moved to Gerrit: https://gwt-review.googlesource.com/3834

http://gwt-code-reviews.appspot.com/1822803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[gwt-contrib] Re: Add NavigableSet, NavigableMap to GWT and retrofit TreeMap and TreeSet to implement it. (issue1839803)

[t . broyer]

Patch moved to Gerrit: https://gwt-review.googlesource.com/3650

http://gwt-code-reviews.appspot.com/1839803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[gwt-contrib] Re: Make PopupPanel#setAnimationType public (issue1893803)

[t . broyer]

If you still care about it, could you please move the patch over to
Gerrit?
http://www.gwtproject.org/makinggwtbetter.html

http://gwt-code-reviews.appspot.com/1893803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[gwt-contrib] Re: Add NavigableSet, NavigableMap to GWT and retrofit TreeMap and TreeSet to implement it. (issue1839803)

[t . broyer]

Can someone move that patch over to Gerrit? Would you prefer that I do
it?

(BTW, any reason this hadn't been reviewed yet?)

http://gwt-code-reviews.appspot.com/1839803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[gwt-contrib] Re: Fixes #8036, properly sorting of places in PlaceHistoryGeneratorContext (issue1895803)

[t . broyer]

On 2013/05/28 08:52:04, larsaaslin wrote:


Can I run only MostToLeastDerivedPlaceTypeComparatorTest with an ant

command?

Yes ant test
-Dgwt.junit.testcase.includes=**/MostToLeastDerivedPlaceTypeComparatorTest.class
or cd user  ant test.nongwt
-Dgwt.nongwt.testcase.includes=**/MostToLeastDerivedPlaceTypeComparatorTest.class


Only getting the following errors when I run it from Eclipse:



java.lang.NullPointerException
at com.google.gwt.dev.util.Util.copyNoClose(Util.java:265)
at com.google.gwt.dev.util.Util.copy(Util.java:186)
at com.google.gwt.dev.util.Util.readStreamAsString(Util.java:713)
at


com.google.gwt.place.rebind.RealJavaResource.getContent(RealJavaResource.java:41)

Make sure you include the sources into the classpath.

http://gwt-code-reviews.appspot.com/1895803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[gwt-contrib] Re: Issue 5525 - Allow EntityProxyId as an argument to a Request Factory service method (issue1899803)

[t . broyer]

Sounds good!


http://gwt-code-reviews.appspot.com/1899803/diff/5005/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java
File
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java
(right):

http://gwt-code-reviews.appspot.com/1899803/diff/5005/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java#newcode1493
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java:1493:
req.simpleFooRequest().fetchDoubleReference().fire(
Use findSimpleFoo(1L) instead.

http://gwt-code-reviews.appspot.com/1899803/

--
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups Google Web Toolkit Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[gwt-contrib] Re: Fixes #8036, properly sorting of places in PlaceHistoryGeneratorContext (issue1895803)

[t . broyer]

We just need someone with commit rights. Calling in Matthew (at random),
who will delegate if needed.

http://gwt-code-reviews.appspot.com/1895803/

--
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups Google Web Toolkit Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[gwt-contrib] Re: ie9 bug fixes. (issue1383809)

[t . broyer]

Marius: I agree with your comments, but this change has actually been
merged already (despite not being closed here).

Please file an issue and possibly provide a patch (preferably to Gerrit
rather than Rietveld)

As for the reasons of the method overrides, it probably has to do with
this:
http://stackoverflow.com/questions/2717252/document-body-scrolltop-is-always-0-in-ie-even-when-scrolling
(though I haven't tested in IE9 how it actually behaves; keeping in mind
that the DOMImplIE9 is only used for IE9 in IE9 Standards mode, i.e.
documentMode = 9)

http://gwt-code-reviews.appspot.com/1383809/

--
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups Google Web Toolkit Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[gwt-contrib] Fixes #8036, properly sorting of places in PlaceHistoryGeneratorContext (issue1895803)

[t . broyer]

LGTM

http://gwt-code-reviews.appspot.com/1895803/

--
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups Google Web Toolkit Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[gwt-contrib] Re: Make PopupPanel#setAnimationType public (issue1893803)

[t . broyer]

http://gwt-code-reviews.appspot.com/1893803/

--
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups Google Web Toolkit Contributors group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[gwt-contrib] Re: DateBox (DatePicker) still allows user to choose disabled dates with mouse click (issue1885803)

[t . broyer]

According to
http://code.google.com/p/google-web-toolkit/issues/detail?id=7919 it
worked the way you describe in GWT 2.4 so let's go ahead and fix this.
Apparently the regression was introduced in
https://code.google.com/p/google-web-toolkit/source/detail?r=11174

Daniel: how's 2.5.1 going? is it too late to put this patch in? it's a
regression so I'd like to see it fixed as early as possible.


http://gwt-code-reviews.appspot.com/1885803/diff/3001/user/src/com/google/gwt/user/datepicker/client/CellGridImpl.java
File user/src/com/google/gwt/user/datepicker/client/CellGridImpl.java
(right):

http://gwt-code-reviews.appspot.com/1885803/diff/3001/user/src/com/google/gwt/user/datepicker/client/CellGridImpl.java#newcode271
user/src/com/google/gwt/user/datepicker/client/CellGridImpl.java:271: if
(!cell.isEnabled()) {
Apparently (form the following lines), this method can be called with
'null'.

Replacing the cell.isEnabled() check with isActive(cell) (which does the
null-check) won't help here as we would return when cell==null,
introducing a change in behavior.

It seems to me like all the calls to setSelected in GWT's codebase (I
suspect Google extend/uses this class internally in their own projects)
but one are guarded by an 'if (isActive(cell))' check, so let's add that
check around the call that's missing it and not change setSelected
behavior's.

http://gwt-code-reviews.appspot.com/1885803/

--
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups Google Web Toolkit Contributors group.

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[gwt-contrib] Re: Issue 7112 (issue1649803)

[t . broyer]

On 2013/01/28 21:06:02, mdempsky wrote:

On 2013/01/12 15:35:19, tbroyer wrote:
 We're in dev-time code so we can use Guava (the re-packaged one) and

it's

 Objects.equal(a, b) which takes care of nulls.



I don't think that's true.  This code still needs to be translatable

to

JavaScript to support running tests in production mode


D'oh, you're right indeed!

Still, adding a 'private boolean equals(Object x, Object y)' method
would make the code easier to read.

http://gwt-code-reviews.appspot.com/1649803/

--
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups Google Web Toolkit Contributors group.

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[gwt-contrib] Re: DateBox (DatePicker) still allows user to choose disabled dates with mouse click (issue1885803)

[t . broyer]

Can you please make your patch from trunk/ rather than
trunk/user/src/com/google/gwt/user/datepicker/client/ ?

That said, there are other implications than just selecting from the
calendar view:
 - what if I setValue() with a disabled date? (I mean, what should be
the expected behavior)
 - what if I enter a disabled date in a DateBox?

I'm not sure CellGridImpl is the right place to fix it either: what if I
use a custom CalendarView? The question actually is: who is responsible
for ignoring the events for a disabled date? is it the CalendarView
implementation? or the DatePicker/DateBox?

I cannot publish comments on the patch due to the bogus Base URL, so
here's my comment:
Shouldn't the isEnabled() check be moved into setSelected() ? There are
a couple other calls to setSelected() that you didn't guard with
isEnabled. I don't know when/how they're triggered but it seems bogus to
me.
This is assuming CellGridImpl is the right place to fix this issue
though (see above)

http://gwt-code-reviews.appspot.com/1885803/

--
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Add experimental GWT.unloadModule() function. (issue1827804)

[t . broyer]

LGTM. Nothing controversial I believe.

This is a pretty big change though, so maybe an additional pair of eyes
would be better (but maybe you also have an internal review ongoing?)


http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/core/client/impl/Impl.java
File user/src/com/google/gwt/core/client/impl/Impl.java (right):

http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/core/client/impl/Impl.java#newcode279
user/src/com/google/gwt/core/client/impl/Impl.java:279: if
(unloadSupport.isUnloadSupported()  Impl.isModuleUnloaded()) {
On 2012/10/08 22:27:38, cromwellian wrote:

On 2012/09/14 09:24:16, tbroyer wrote:
 Impl. qualifier is superfluous



Done.


Apparently not.

http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/core/client/impl/UnloadSupportEnabled.java
File user/src/com/google/gwt/core/client/impl/UnloadSupportEnabled.java
(right):

http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/core/client/impl/UnloadSupportEnabled.java#newcode93
user/src/com/google/gwt/core/client/impl/UnloadSupportEnabled.java:93:
GWT.UncaughtExceptionHandler uncaughtExceptionHandler =
GWT.getUncaughtExceptionHandler();
On 2012/09/14 17:23:41, cromwellian wrote:

I agree with the umbrella exception, but this method is also called

from JSNI

directly in some spots. We don't want a bubbling exception to prevent

removal

from disposeables (it could be moved before the call to d.dispose())

but more

generally, probaly don't want all of the try/catch logic in JS.



I could split this into a version that has the try catch (for JSNI

calls) and

leave a version for Java callers without.


Maybe add a comment at least, documenting why it's needed (called
directly from JSNI outside $entry scope), so that someone revisiting
this code in a few months won't be tempted to remove it.

I otherwise like the idea of splitting this method (most probably rather
add Impl.disposeWithoutError with the try/catch, called from JSNI, and
let Impl.dispose simply call UnloadSupport#dispose, to be called from
Java, and remove the try/catch from here; and of course add try/catch in
disposeAll to build an UmbrellaException)

http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/user/client/ui/PotentialElement.java
File user/src/com/google/gwt/user/client/ui/PotentialElement.java
(right):

http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/user/client/ui/PotentialElement.java#newcode49
user/src/com/google/gwt/user/client/ui/PotentialElement.java:49:
$wnd.GwtPotentialElementShim = null;
On 2012/10/08 22:27:38, cromwellian wrote:

I removed it for now, but I think I will have to use a global
eval/set-timeout/script-tag-inject trick to have it constructed via a

string in

the host page. The shim code is clearly residing in the IFRAME's

module

source, and letting the host page hold a reference to it will likely

cause the

entire source to be retained (because of the way the VM preserves

debugging

info)


Does it really need to be a global in $wnd? Can't it be in 'window'
instead and/or possibly use the module name as a prefix/suffix? (similar
to DOMImplTrident's window['__gwt_dispatchEvent_' + moduleName])

http://gwt-code-reviews.appspot.com/1827804/diff/10001/user/src/com/google/gwt/core/client/GWT.java
File user/src/com/google/gwt/core/client/GWT.java (right):

http://gwt-code-reviews.appspot.com/1827804/diff/10001/user/src/com/google/gwt/core/client/GWT.java#newcode88
user/src/com/google/gwt/core/client/GWT.java:88: public static void
exportUnloadModule() {
How about a com.google.gwt.core.UnloadEnabled module that defines
gwt.unloadEnabled to true and defines an entry-point that automatically
calls exportUnloadModule?
Or possibly simply define the entry-point in Core.gwt.xml
(exportUnloadModule will be a no-op when unload support is disabled so
it should be compiled out anyway)

http://gwt-code-reviews.appspot.com/1827804/diff/10001/user/src/com/google/gwt/core/client/GWT.java#newcode293
user/src/com/google/gwt/core/client/GWT.java:293: * cleaned up. This
memory is not typically called by the GWT module itself, but exported so
that another module
Did you mean 'method' rather than 'memory' here?

http://gwt-code-reviews.appspot.com/1827804/diff/10001/user/src/com/google/gwt/core/client/impl/Impl.java
File user/src/com/google/gwt/core/client/impl/Impl.java (right):

http://gwt-code-reviews.appspot.com/1827804/diff/10001/user/src/com/google/gwt/core/client/impl/Impl.java#newcode248
user/src/com/google/gwt/core/client/impl/Impl.java:248: private static
void disposeAll() {
There should probably be a comment here explaining where this method is
called from?

http://gwt-code-reviews.appspot.com/1827804/diff/10001/user/src/com/google/gwt/core/client/impl/UnloadSupportEnabled.java
File user/src/com/google/gwt/core/client/impl/UnloadSupportEnabled.java
(right):

[gwt-contrib] Re: Update the cell table builder to allow for rendering the cell attribute somewhere other than the... (issue1879803)

[t . broyer]

On 2013/01/22 15:46:27, arobison wrote:

Removed the new cell table builder and reworked to make the change in
AbstractCellTable instead.  Removed the isKeyboardSelectable stuff.


Can you update the description to match the actual changes? (I must say
I'm a bit lost).
If it indeed is a fix for issue 7508, then include Fixes issue 7508 (I
believe the script you use to then actually commit the change uses the
description as the commit message, and having a xref to the issue from
the commit helps us maintaining issue status up-to-date)
https://code.google.com/p/google-web-toolkit/issues/detail?id=7508

Not sure what you mean by HTML-table implementations and non-HTML
implementations in the comments too.

http://gwt-code-reviews.appspot.com/1879803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Support withCredentials attribute for XHR requests (Chrome 3+, Firefox 3.5+, Opera12+, Safari 4+... (issue1879804)

[t . broyer]

http://gwt-code-reviews.appspot.com/1879804/diff/1/user/src/com/google/gwt/xhr/client/XMLHttpRequest.java
File user/src/com/google/gwt/xhr/client/XMLHttpRequest.java (right):

http://gwt-code-reviews.appspot.com/1879804/diff/1/user/src/com/google/gwt/xhr/client/XMLHttpRequest.java#newcode377
user/src/com/google/gwt/xhr/client/XMLHttpRequest.java:377: if
(withCredentials in this.xmlHttpRequest) {
Is this check really needed? That XHR class is low-level, you're
supposed to know what you're doing when using it. Calling
setWithCredentials() on a non-cross-origin request is probably a
mistake, and I wouldn't really care that it throws in browsers not
supporting CORS (e.g. IE up to and including IE9), and if the browser
doesn't support CORS and you try to do a cross-origin request, then
open() would throw anyway, so it doesn't seem worth it to avoid an
exception in setWithCredentials.

http://gwt-code-reviews.appspot.com/1879804/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Avoid using eval() (issue1882803)

[t . broyer]

(adding jtamplin as a reviewer as it's in I18N)


http://gwt-code-reviews.appspot.com/1882803/diff/1/user/src/com/google/gwt/i18n/client/TimeZoneInfo.java
File user/src/com/google/gwt/i18n/client/TimeZoneInfo.java (right):

http://gwt-code-reviews.appspot.com/1882803/diff/1/user/src/com/google/gwt/i18n/client/TimeZoneInfo.java#newcode45
user/src/com/google/gwt/i18n/client/TimeZoneInfo.java:45: return
JSON.parse(json);
JSON.parse() isn't available everywhere (that GWT supports).

Use JsonUtils.safeEval() instead, it'll use JSON.parse if available, and
fallback to eval() after checking it's safeToEval().

http://gwt-code-reviews.appspot.com/1882803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Support withCredentials attribute for XHR requests (Chrome 3+, Firefox 3.5+, Opera12+, Safari 4+... (issue1879804)

[t . broyer]

FYI, an issue related to this change:
https://code.google.com/p/google-web-toolkit/issues/detail?id=7677

http://gwt-code-reviews.appspot.com/1879804/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Avoid using eval() (issue1882803)

[t . broyer]

Superseded by http://gwt-code-reviews.appspot.com/1883803

miichal, you can close this review.

http://gwt-code-reviews.appspot.com/1882803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix issue 6959. (issue1587803)

[t . broyer]

http://gwt-code-reviews.appspot.com/1587803/diff/5002/user/src/com/google/gwt/editor/client/adapters/EditorSource.java
File user/src/com/google/gwt/editor/client/adapters/EditorSource.java
(right):

http://gwt-code-reviews.appspot.com/1587803/diff/5002/user/src/com/google/gwt/editor/client/adapters/EditorSource.java#newcode60
user/src/com/google/gwt/editor/client/adapters/EditorSource.java:60: *
For backwards compatibility with GWT 2.5.0 and earlier, the default
implemtation calls
On 2013/01/15 02:55:22, skybrian wrote:

sp: implementation


Done.

http://gwt-code-reviews.appspot.com/1587803/diff/5002/user/src/com/google/gwt/editor/client/adapters/HasDataEditor.java
File user/src/com/google/gwt/editor/client/adapters/HasDataEditor.java
(right):

http://gwt-code-reviews.appspot.com/1587803/diff/5002/user/src/com/google/gwt/editor/client/adapters/HasDataEditor.java#newcode44
user/src/com/google/gwt/editor/client/adapters/HasDataEditor.java:44:
return new IndexedEditorT(-1, null);
On 2013/01/15 02:55:22, skybrian wrote:

If IndexedEditor's index is -1 then getValue() always returns null and
setValue() probably throws an exception eventually in data.setRowData

(I didn't

trace it). It seems like it would be clearer to return an anonymous

subclass of

LeafValueEditor that implements getValue() and setValue() to do this

directly?

I'm assuming setValue() shouldn't be called at all, not sure about

getValue.

I was assuming getValue/setValue wouldn't be called, but user code can
actually call them (from the EditorVisitor passed to
EditorContext#traverseSyntheticCompositeEditor). I can't see any use
case for that, but still. So I fixed IndexedEditor to no longer throw if
data is null.


Then dispose() can do an instanceof check and ignore this instance. Or

perhaps

keep it in a constant and use ==.


Actually, dispose() won't be called; that was a leftover from a previous
iteration where I only moved the create(0) to createEditorForTraversal;
I then moved both create(0) and dispose() so, in the case of
HasDataEditor, dispose is no longer called for the synthetic editor.

http://gwt-code-reviews.appspot.com/1587803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix issue 6959. (issue1587803)

[t . broyer]

http://gwt-code-reviews.appspot.com/1587803/diff/1/user/src/com/google/gwt/editor/client/adapters/EditorSource.java
File user/src/com/google/gwt/editor/client/adapters/EditorSource.java
(right):

http://gwt-code-reviews.appspot.com/1587803/diff/1/user/src/com/google/gwt/editor/client/adapters/EditorSource.java#newcode42
user/src/com/google/gwt/editor/client/adapters/EditorSource.java:42:
public abstract E create(int index);
On 2013/01/14 21:16:49, skybrian wrote:

Instead of using -1 to indicate a synthetic editor, it seems cleaner

to add a

new createEditorForTraversal() method. The default implementation

could just

call create(0) for backward compatibility, but HasDataEditor could

implement it

to either set a flag on IndexedEditor or alternately use a different

subclass

altogether.


Indeed! Why didn't I think of that?!

Thanks Brian, will update the patch soon.

http://gwt-code-reviews.appspot.com/1587803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix issue 6959. (issue1587803)

[t . broyer]

http://gwt-code-reviews.appspot.com/1587803/diff/1/user/src/com/google/gwt/editor/client/adapters/EditorSource.java
File user/src/com/google/gwt/editor/client/adapters/EditorSource.java
(right):

http://gwt-code-reviews.appspot.com/1587803/diff/1/user/src/com/google/gwt/editor/client/adapters/EditorSource.java#newcode42
user/src/com/google/gwt/editor/client/adapters/EditorSource.java:42:
public abstract E create(int index);
On 2013/01/14 21:16:49, skybrian wrote:

Instead of using -1 to indicate a synthetic editor, it seems cleaner

to add a

new createEditorForTraversal() method. The default implementation

could just

call create(0) for backward compatibility, but HasDataEditor could

implement it

to either set a flag on IndexedEditor or alternately use a different

subclass

altogether.


Done.

http://gwt-code-reviews.appspot.com/1587803/diff/1/user/src/com/google/gwt/editor/client/adapters/HasDataEditor.java
File user/src/com/google/gwt/editor/client/adapters/HasDataEditor.java
(right):

http://gwt-code-reviews.appspot.com/1587803/diff/1/user/src/com/google/gwt/editor/client/adapters/HasDataEditor.java#newcode56
user/src/com/google/gwt/editor/client/adapters/HasDataEditor.java:56:
((IndexedEditorT) editor).setIndex(index);
On 2013/01/14 21:16:49, skybrian wrote:

assert index =0?


Done.

http://gwt-code-reviews.appspot.com/1587803/diff/1/user/src/com/google/gwt/editor/client/adapters/HasDataEditor.java#newcode60
user/src/com/google/gwt/editor/client/adapters/HasDataEditor.java:60:
static class IndexedEditorQ implements LeafValueEditorQ {
On 2013/01/14 21:16:49, skybrian wrote:

Perhaps IndexedEditor should be private? I see no usages other than in

this

class.


Done.

http://gwt-code-reviews.appspot.com/1587803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix issue 6959. (issue1587803)

[t . broyer]

Ping; would be great to have it in 2.5.1! (IIRC, we initially talked
about including it in 2.5.0 and finally decided to postpone it to the
next release)

http://gwt-code-reviews.appspot.com/1587803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Issue 7112 (issue1649803)

[t . broyer]

http://gwt-code-reviews.appspot.com/1649803/diff/1/src/test/java/com/google/gwt/junit/client/impl/JUnitHost.java
File src/test/java/com/google/gwt/junit/client/impl/JUnitHost.java
(right):

http://gwt-code-reviews.appspot.com/1649803/diff/1/src/test/java/com/google/gwt/junit/client/impl/JUnitHost.java#newcode152
src/test/java/com/google/gwt/junit/client/impl/JUnitHost.java:152:
We're in dev-time code so we can use Guava (the re-packaged one) and
it's Objects.equal(a, b) which takes care of nulls.

http://gwt-code-reviews.appspot.com/1649803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: SplitLayoutPanel needs to set the associated Splitter to hidden when a child is set to hidden (issue1880804)

[t . broyer]

http://gwt-code-reviews.appspot.com/1880804/diff/1/user/src/com/google/gwt/user/client/ui/SplitLayoutPanel.java
File user/src/com/google/gwt/user/client/ui/SplitLayoutPanel.java
(right):

http://gwt-code-reviews.appspot.com/1880804/diff/1/user/src/com/google/gwt/user/client/ui/SplitLayoutPanel.java#newcode385
user/src/com/google/gwt/user/client/ui/SplitLayoutPanel.java:385: if
(layoutData.direction != Direction.CENTER) {
In other parts of the class (e.g. setWidgetMinSize) the fact that
getAssociatedSplitter returns 'null' is used to detect the center
widget. This would simplify this method a lot:

   public void setWidgetHidden(Widget widget, boolean hidden) {
 super.setWidgetHidden(widget, hidden);
 Splitter splitter = getAssociatedSplitter(widget);
 if (splitter != null) {
   // The splitter is null for the center element.
   super.setWidgetHidden(splitter, hidden);
 }
   }

http://gwt-code-reviews.appspot.com/1880804/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: SplitLayoutPanel needs to set the associated Splitter to hidden when a child is set to hidden (issue1880804)

[t . broyer]

LGTM


http://gwt-code-reviews.appspot.com/1880804/diff/4001/user/src/com/google/gwt/user/client/ui/SplitLayoutPanel.java
File user/src/com/google/gwt/user/client/ui/SplitLayoutPanel.java
(right):

http://gwt-code-reviews.appspot.com/1880804/diff/4001/user/src/com/google/gwt/user/client/ui/SplitLayoutPanel.java#newcode388
user/src/com/google/gwt/user/client/ui/SplitLayoutPanel.java:388:
super.setWidgetHidden(widget, hidden);
Note that if you move this line above the splitter, you don't even need
the assertIsChild, as it's done in setWidgetHidden anyway.

In general I prefer it that way: first do what's supposed to be done,
then add the other things. If the normal processing throws, your
additions won't run; that's what you want, and you get it for free. You
don't need to synchronize your preconditions with the ones of the other
class in the event they'd change a bit, etc. it just works the way it's
supposed to work.

YMMV of course, and the code you have here should work great. So no need
to change it unless someone else asks you, or you want to change it.

http://gwt-code-reviews.appspot.com/1880804/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Too many open files when generating ClientBundles (issue1880803)

[t . broyer]

http://gwt-code-reviews.appspot.com/1880803/diff/7001/dev/core/src/com/google/gwt/util/tools/Utility.java
File dev/core/src/com/google/gwt/util/tools/Utility.java (right):

http://gwt-code-reviews.appspot.com/1880803/diff/7001/dev/core/src/com/google/gwt/util/tools/Utility.java#newcode54
dev/core/src/com/google/gwt/util/tools/Utility.java:54: } catch
(IOException e) {
On 2013/01/04 10:00:17, jens wrote:

On 2013/01/04 02:26:08, tbroyer wrote:
 On 2013/01/04 01:31:40, jens wrote:
  How about logging this exception as it is an I/O error and could

indicate

  failing hardware?
  Personally I think completely ignoring this exception just because

you can

not
  recover from it is bad practice.

 While I agree on principle, I think this is best left for another

CL.

 Guava has a nice Closeables utility [1] that we might want to use,

but that

 means replacing all uses of Utility.close(Closeable) with
 Closeables.close(Closeable,boolean) and adding a boolean in these

try/finally

to
 track whether to swallow the exception.

 [1]



http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/com/google/common/io/Closeables.html#close%2528java.io.Closeable,

 boolean)



I agree that introducing Closeables.close(..,  ..) is better done in

another CL.



But this method is touched now and replaces all other Utility.close()

methods

thus making it a good place to actually react to this IOException in

the mean

time. Imagine GWT uses Utility.close(BufferedOutputStream) somewhere:

calling

BufferedOutputStream.close() will result in flushing remaining buffers

and if

that fails you probably have written out something broken/incomplete

without

detecting it.



In fact that is the case for FilterOutputStream and all its sub

classes:

http://docs.oracle.com/javase/6/docs/api/java/io/FilterOutputStream.html#close%28)

So you're proposing to rather add a close(ImageInputStream) method than
factor all those close() methods into a single close(Closeable) method?

http://gwt-code-reviews.appspot.com/1880803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Too many open files when generating ClientBundles (issue1880803)

[t . broyer]

LGTM

(BTW why was the GWT-Contrib group removed from CC?)


http://gwt-code-reviews.appspot.com/1880803/diff/1/user/src/com/google/gwt/resources/rg/ImageBundleBuilder.java
File user/src/com/google/gwt/resources/rg/ImageBundleBuilder.java
(right):

http://gwt-code-reviews.appspot.com/1880803/diff/1/user/src/com/google/gwt/resources/rg/ImageBundleBuilder.java#newcode778
user/src/com/google/gwt/resources/rg/ImageBundleBuilder.java:778: if
(imageStream != null) {
On 2013/01/04 01:15:25, Colin Alworth wrote:

On 2013/01/03 23:46:58, tbroyer wrote:
 There should be a try/catch for each stream; and we should probably

close the

 MemoryCacheImageInputStream before the underlying InputStream.
Using Utility.close deals with the separate try/catch issue, and I

believe they

are already in the correct order?


Ah yes sorry, maybe the variable names should be changed then, I thought
'imageStream' would be the ImageInputStream and 'input' were the
InputStream. How about 'is' for the InputStream?

http://gwt-code-reviews.appspot.com/1880803/diff/7001/dev/core/src/com/google/gwt/util/tools/Utility.java
File dev/core/src/com/google/gwt/util/tools/Utility.java (right):

http://gwt-code-reviews.appspot.com/1880803/diff/7001/dev/core/src/com/google/gwt/util/tools/Utility.java#newcode54
dev/core/src/com/google/gwt/util/tools/Utility.java:54: } catch
(IOException e) {
On 2013/01/04 01:31:40, jens wrote:

How about logging this exception as it is an I/O error and could

indicate

failing hardware?
Personally I think completely ignoring this exception just because you

can not

recover from it is bad practice.


While I agree on principle, I think this is best left for another CL.
Guava has a nice Closeables utility [1] that we might want to use, but
that means replacing all uses of Utility.close(Closeable) with
Closeables.close(Closeable,boolean) and adding a boolean in these
try/finally to track whether to swallow the exception.

[1]
http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/com/google/common/io/Closeables.html#close(java.io.Closeable,
boolean)

http://gwt-code-reviews.appspot.com/1880803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Fix using element after removing it from map (issue1877803)

[t . broyer]

Only suggestions, we probably need microbenchmarks to choose between the
alternatives, as AbstractMap is used a lot within applications (at least
one per Widget, then an additional one per event type with a registered
handler).


http://gwt-code-reviews.appspot.com/1877803/diff/1/user/super/com/google/gwt/emul/java/util/AbstractMap.java
File user/super/com/google/gwt/emul/java/util/AbstractMap.java (right):

http://gwt-code-reviews.appspot.com/1877803/diff/1/user/super/com/google/gwt/emul/java/util/AbstractMap.java#newcode144
user/super/com/google/gwt/emul/java/util/AbstractMap.java:144: for
(IteratorEntryK, V iter = entrySet().iterator(); iter.hasNext();) {
Instead of duplicating the code from implFindEntry, how about changing
implFindEntry to return the entry's value instead? (using a marker value
to signal that no entry was found).

containsKey would then be:

   return implFindEntry(key, false) != ABSENT;

and get and remove would use:

   return value == ABSENT ? null : value;

The downside to that approach is that it introduces a clinit if ABSENT
is made static, or an ABSENT instance per AbstractMap otherwise.

Alternatively, implFindEntry could be changed to:

   if (remove) {
 entry = new MapEntryImplK, V(entry.getKey(), entry.getValue());
 iter.remove();
   }

only adding a small runtime overhead on removal.

http://gwt-code-reviews.appspot.com/1877803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: GWT does not normalize IE mangling from http 204 code to 1223 (Issue 5031) (issue1875803)

[t . broyer]

And +1 to everything Matthew already said.


http://gwt-code-reviews.appspot.com/1875803/diff/1/user/src/com/google/gwt/http/client/RequestImpl.java
File user/src/com/google/gwt/http/client/RequestImpl.java (right):

http://gwt-code-reviews.appspot.com/1875803/diff/1/user/src/com/google/gwt/http/client/RequestImpl.java#newcode22
user/src/com/google/gwt/http/client/RequestImpl.java:22: public class
RequestImpl {
All methods are package-protected, so maybe the class should be
package-protected too?

Alternatively, make everything public and move the class to an impl
subpackage.

http://gwt-code-reviews.appspot.com/1875803/diff/1/user/src/com/google/gwt/http/client/RequestImpl.java#newcode75
user/src/com/google/gwt/http/client/RequestImpl.java:75: Header[]
getHeaders(XMLHttpRequest xmlHttp) {
Why did you create getHeaders and isResponseReady if they're not going
to be overridden?

http://gwt-code-reviews.appspot.com/1875803/diff/1/user/src/com/google/gwt/http/client/RequestImplIE69.java
File user/src/com/google/gwt/http/client/RequestImplIE69.java (right):

http://gwt-code-reviews.appspot.com/1875803/diff/1/user/src/com/google/gwt/http/client/RequestImplIE69.java#newcode24
user/src/com/google/gwt/http/client/RequestImplIE69.java:24: public
class RequestImplIE69 extends RequestImpl {
Does that mean this is no longer an issue in IE10?

I'm not found of the IE69 naming scheme. AFAICT we've never used such
a rule. I'd rather name the class IE, even if IE10+ doesn't need it.

http://gwt-code-reviews.appspot.com/1875803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Dataprovider only updating visible displays

[Alexander T]

Hi.

We have multiple DataGrids sharing the same DataProvider. What seems to 
happen is that only the visible DataGrids get updated when the data from 
the provider changes. Other displays residing in other tabs in our tab 
panes seems to hold the data, but when you change to their tabs they all 
show up blank. Looking in the DOM inspector, you can see that they have no 
data, only headers. As soon as you manipulate the lists, by for instance 
asking them to select a specific row, they are instantaneously updated to 
the correct state. This happens with both Async and List data provider. I 
was wondering if anyone else is seeing this behavior before I proceed with 
trying to debug the code and reproduce a test case. We're running 2.5.0.

Thank you

Alexander T

-- 
You received this message because you are subscribed to the Google Groups 
Google Web Toolkit group.
To view this discussion on the web visit 
https://groups.google.com/d/msg/google-web-toolkit/-/lFe5tT3tUmYJ.
To post to this group, send email to google-web-toolkit@googlegroups.com.
To unsubscribe from this group, send email to 
google-web-toolkit+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/google-web-toolkit?hl=en.

[gwt-contrib] Re: added return values to addClassName and removeClassName to let the caller know if the style list... (issue1873803)

[t . broyer]

http://gwt-code-reviews.appspot.com/1873803/diff/1/user/src/com/google/gwt/dom/client/Element.java
File user/src/com/google/gwt/dom/client/Element.java (right):

http://gwt-code-reviews.appspot.com/1873803/diff/1/user/src/com/google/gwt/dom/client/Element.java#newcode104
user/src/com/google/gwt/dom/client/Element.java:104:
setClassName(oldClassName +   + className);
Would it break anything if we unconditionally did this even if
oldClassName is the empty string ?

(BTW, we now require Java 6 and have emulated isEmpty() for long, so we
could also use it instead of length()0)

http://gwt-code-reviews.appspot.com/1873803/diff/1/user/src/com/google/gwt/dom/client/Element.java#newcode565
user/src/com/google/gwt/dom/client/Element.java:565: static int
indexOfName(String nameList, String name) {
I wonder if this wouldn't be faster:

   return (  + nameList +  ).indexOf(  + name +  );

It sure is easier to read IMO ;-)

http://gwt-code-reviews.appspot.com/1873803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Added a method that exposes DataGrid's ScrollPanel, to allow some customization by subclasses. (issue1864803)

[t . broyer]

On 2012/11/02 16:37:08, rkj wrote:

On 2012/11/02 02:14:47, tbroyer wrote:
 FYI, there's an issue for that:
 http://code.google.com/p/google-web-toolkit/issues/detail?id=6865
 (understand: whose status should be updated)



Thanks, that's exactly this issue. I have a UI problem though - I do

not see how

to link this change with the bug.


FYI, there's another issue which is somewhat related:
http://code.google.com/p/google-web-toolkit/issues/detail?id=7589

(and I've updated the issue status)

http://gwt-code-reviews.appspot.com/1864803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Added a method that exposes DataGrid's ScrollPanel, to allow some customization by subclasses. (issue1864803)

[t . broyer]

FYI, there's an issue for that:
http://code.google.com/p/google-web-toolkit/issues/detail?id=6865
(understand: whose status should be updated)

http://gwt-code-reviews.appspot.com/1864803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix for Issue 7754: Type mismatch compile time exception when Editor uses a specific type parameter (issue1860804)

[t . broyer]

You're basically trading a compile-time error for a runtime error. I
must say I don't quite like it.

IIRC, the Editor framework mandates that the editor type (from the
EditorT parameterization) and the edited type (from the property or
field, or the EditorDriver parameterization) must be the same.

See http://code.google.com/p/google-web-toolkit/issues/detail?id=6016
and the linked discussion from GWT-Contrib.

http://gwt-code-reviews.appspot.com/1860804/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Split ValidationTool.exec() into two methods so alternative (issue1859803)

[t . broyer]

http://gwt-code-reviews.appspot.com/1859803/diff/2001/user/src/com/google/web/bindery/requestfactory/apt/ValidationTool.java
File
user/src/com/google/web/bindery/requestfactory/apt/ValidationTool.java
(right):

http://gwt-code-reviews.appspot.com/1859803/diff/2001/user/src/com/google/web/bindery/requestfactory/apt/ValidationTool.java#newcode216
user/src/com/google/web/bindery/requestfactory/apt/ValidationTool.java:216:
protected JavaCompiler getJavaCompiler() {
On 2012/10/24 20:52:30, skybrian wrote:

Not a big deal, but I usually prefer to introduce a field and

constructor arg

for configuration, to avoid starting a class hierarchy.


+1

http://gwt-code-reviews.appspot.com/1859803/diff/6001/user/src/com/google/web/bindery/requestfactory/apt/ValidationTool.java
File
user/src/com/google/web/bindery/requestfactory/apt/ValidationTool.java
(right):

http://gwt-code-reviews.appspot.com/1859803/diff/6001/user/src/com/google/web/bindery/requestfactory/apt/ValidationTool.java#newcode212
user/src/com/google/web/bindery/requestfactory/apt/ValidationTool.java:212:
public void run(String[] args) throws IOException {
Er, why this method? Couldn't the System.exit be in the main()?

  System.exit(new ValidationTool().exec(args) ? 0 : -1);

or

  boolean successful = new ValidationTool().exec(args);
  System.exit(successful ? 0 : -1);

http://gwt-code-reviews.appspot.com/1859803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Replace instances of element.setInnerHTML(safeHtml.asString()) (issue1857803)

[t . broyer]

D'oh, where was I when Element#setInnerSafeHtml was added? Didn't know
it even existed!

LGTM++

Except you missed one that spans 2 lines in ImageLoadingCell.
(caught using grep -R -A 1 'setInnerHTML' user/src/com/google/gwt/
|grep asString |grep -v setInnerHTML ;-) )

http://gwt-code-reviews.appspot.com/1857803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Replace instances of element.setInnerHTML(safeHtml.asString()) (issue1857803)

[t . broyer]

On 2012/10/19 12:50:40, tbroyer wrote:

Except you missed one that spans 2 lines in ImageLoadingCell.
(caught using grep -R -A 1 'setInnerHTML' user/src/com/google/gwt/

|grep

asString |grep -v setInnerHTML ;-) )


Ah, missed it in the patch; you already caught it, great!

http://gwt-code-reviews.appspot.com/1857803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Be stricter about quoting JSON strings in AutoBeans. In particular, (issue1853803)

[t . broyer]

On 2012/10/13 03:42:46, cromwellian wrote:

One option is to have an extra method like encodeForHtmlContext()

where you only

pay the tax in that circumstance.


Would it cost really that much to post-process the output from AutoBeans
(possibly depending on context)?

For instance, using OWASP ESAPI [1]

   String json = AutoBeanCodex.encode(bean).getPayload();
   string escaped = new DefaultEncoder().encodeForJavaScript(json);

[1]
http://owasp-esapi-java.googlecode.com/svn/trunk_doc/latest/org/owasp/esapi/reference/DefaultEncoder.html

http://gwt-code-reviews.appspot.com/1853803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Be stricter about quoting JSON strings in AutoBeans. In particular, (issue1853803)

[t . broyer]

Should HTML-specific escaping be done on top of the value produced by
AutoBeans? Do we really want to pay the tax (each one of =' is
replaced by 6 chars!) for each AutoBean-based exchanged over the wire
(for example, RequestFactory uses Base64-encoding for its stableId and
version, which makes use =).

Also, the dangerous HTML chars depend on context: for a script, I
don't think any of those char is dangerous (given than
JSONObject.quote already escapes /, producing a \/script). For
another element (e.g. a span style=display:none), only  and  are
actual issues. And for attribute values, basically only quotes and  are
issues, possibly  and  though I believe they've only been issues with
really old browsers. Moreover, for all those chars, the HTML escape is
shorter than the JS escape (lt; vs. \u003C, #34; vs \u0022)

The comments below assume the above is refuted somehow.


http://gwt-code-reviews.appspot.com/1853803/diff/1/user/src/com/google/web/bindery/autobean/shared/impl/StringQuoter.java
File
user/src/com/google/web/bindery/autobean/shared/impl/StringQuoter.java
(right):

http://gwt-code-reviews.appspot.com/1853803/diff/1/user/src/com/google/web/bindery/autobean/shared/impl/StringQuoter.java#newcode78
user/src/com/google/web/bindery/autobean/shared/impl/StringQuoter.java:78:
case '/': sb.append(\\/); break;
I don't think / needs escaping anymore if we escape . I guess it
was only escaped to avoid outputing /script that could close an HTML
script, but if  is escaped, it'd become \u003C/script where /
doesn't need to also be escaped.

http://gwt-code-reviews.appspot.com/1853803/diff/1/user/src/com/google/web/bindery/autobean/shared/impl/StringQuoter.java#newcode86
user/src/com/google/web/bindery/autobean/shared/impl/StringQuoter.java:86:
if (c  ' ' || c = 128 || DANGEROUS_HTML_CHARS.indexOf(c) != -1) {
Performance-wise, wouldn't it be better to add case lines above?

http://gwt-code-reviews.appspot.com/1853803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix leak in LayoutImplIE6 (issue1601804)

[t . broyer]

On 2012/10/07 02:00:09, rajneeshg wrote:

On 2012/05/21 20:03:28, stephenh wrote:



I am on 2.5.0-rc1. I am still having this issue. Anybody knows when is

this fix

scheduled for.


2.5.1 or 2.6: the version after 2.5.0.

http://gwt-code-reviews.appspot.com/1601804/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Make the AbstractEditorDriverGenerator create Context classes that work with parameterized parents. (issue1836803)

[t . broyer]

LGTM

http://gwt-code-reviews.appspot.com/1836803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Update Maven sample pom.xml files to use maven-compiler-plugin's annotation processing functiona... (issue1828803)

[t . broyer]

http://gwt-code-reviews.appspot.com/1828803/diff/9001/samples/dynatablerf/pom.xml
File samples/dynatablerf/pom.xml (right):

http://gwt-code-reviews.appspot.com/1828803/diff/9001/samples/dynatablerf/pom.xml#newcode120
samples/dynatablerf/pom.xml:120: version2.3.0-1/version
Oh, I'm really sorry I let it slip through: the plugin should be updated
to 2.5.0-rc1 here, or the (unfortunate) gwt-servlet dependency has to be
reintroduced (and possibly the plugin updated to 2.4.0).

And the validation sample should also benefit from this change (it too
apparently needs a bit of love:
https://groups.google.com/d/topic/google-web-toolkit/D2nrOqJeMmA/discussion)

http://gwt-code-reviews.appspot.com/1828803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: In the Chrome plugin, rename src to java for compatibility with (issue1834803)

[t . broyer]

Do you think it would be possible to share the BUILD file(s), or a
stripped-down version of it, even privately?
I'd love to see how it compares to Maven and other build systems.

Now back to the CL: given the move to Git soon, which will require some
changes on your side, is it wise to make such a change now? I mean, MOE
[1][2] for instance can very well sync external src to internal java
(transmogrify as they call it).
On the other hand, given that the new Git repos will be populated from
Google's internal repo, maybe the answer to the above is yes, and take
advantage of the repo move to transmogrify them back to src (along
with rollbacking the changes to the Ant files) outside Google.

[1] http://code.google.com/p/make-open-easy/
[2] http://code.google.com/p/moe-java/

On 2012/09/17 19:13:46, skybrian wrote:

This is a good place for an experiment, since in practice, very few
people build the plugins.



The bigger picture is that we don't particularly like BUILD files that
call ant, since they slow down builds for everyone at Google. There's
no reason to rebuild every GWT app and run all the tests because one
Java file in gwt.user changed that most people don't even use.



Outside Google, people seem to prefer Maven, and I expect we won't
want to use that internally either. So I'd like to see if we can just
build GWT code directly. If Google can move away from ant and open
source moves to Maven, we'll end up with two independent build systems
instead of three that are intertwined. (Or four, if you count the
plugin Makefiles. But I doubt we can get rid of those.)



- Brian



On Mon, Sep 17, 2012 at 11:45 AM,  mailto:j...@jaet.org wrote:
 So why not do it in the BUILD file the same way the rest of GWT

does,

 which also doesn't have a java directory?

 http://gwt-code-reviews.appspot.com/1834803/



http://gwt-code-reviews.appspot.com/1834803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Move GAE Auth functionality from Expenses over the MobileWebApp sample. (issue1829803)

[t . broyer]

I'm sorry I've only started to review the files (over the last few days)
but I have a first question/comment about where this is going:

There are many things that are not needed in the case of MobileWebApp as
the host page is protected behind authentication. Because the user won't
ever see this page when being unauthenticated:
 - LoginWidget does not need to have a dual state (there's no need for a
login link)
 - the username could be inlined in the script generated by the JSP
(that might not work very well with the ApplicationCache/offline though)

Also, there's probably no need to pass MobileWebApp.jsp as an init-param
to the GaeAuthFilter as it could just use getContextPath()
(MobileWebApp.jsp is in the welcome-file-list in the web.xml), or /
(the context path is always / on GAE)

As far as offline is concerned, it might get in our way here, but I'm no
expert in mobile dev. Anyway, it seems to cause us more harm than
anything, and the linker is known not to be great (it will load all
permutations in the client's cache, only to use one of them)

This would greatly simplify the code, at the expense of removing a bunch
of reusable code (LoginWidget, etc.)


http://gwt-code-reviews.appspot.com/1829803/diff/1/samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/client/GaeAuthRequestTransport.java
File
samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/client/GaeAuthRequestTransport.java
(right):

http://gwt-code-reviews.appspot.com/1829803/diff/1/samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/client/GaeAuthRequestTransport.java#newcode18
samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/client/GaeAuthRequestTransport.java:18:
import com.google.gwt.event.shared.EventBus;
Why this change?

http://gwt-code-reviews.appspot.com/1829803/diff/1/samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/client/GaeAuthenticationFailureEvent.java
File
samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/client/GaeAuthenticationFailureEvent.java
(right):

http://gwt-code-reviews.appspot.com/1829803/diff/1/samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/client/GaeAuthenticationFailureEvent.java#newcode18
samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/client/GaeAuthenticationFailureEvent.java:18:
import com.google.gwt.event.shared.EventBus;
Same question: what's the reason behind this change?

http://gwt-code-reviews.appspot.com/1829803/diff/1/samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/client/GaeAuthenticationFailureEvent.java#newcode64
samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/client/GaeAuthenticationFailureEvent.java:64:
* @param state a {@link State} instance
This looks like a leftover from an experiment (remark also applies to
the javadoc on the loginUrl field)

http://gwt-code-reviews.appspot.com/1829803/diff/1/samples/mobilewebapp/src/main/webapp/MobileWebApp.jsp
File samples/mobilewebapp/src/main/webapp/MobileWebApp.jsp (right):

http://gwt-code-reviews.appspot.com/1829803/diff/1/samples/mobilewebapp/src/main/webapp/MobileWebApp.jsp#newcode37
samples/mobilewebapp/src/main/webapp/MobileWebApp.jsp:37:
window['__gwtsample_mobilewebapp__'].loginUrl =
'%=appUrls.getLoginUrl()%';
Wouldn't an object literal be more readable?

window['__gwtsample_mobilewebapp__'] = {
  loginUrl: '%= appUrls.getLoginUrl() %',
  logoutUrl: '%= appUrls.getLogoutUrl() %'
};

(I'd also use var __gwtsample_mobilewebapp__ = instead of
window['__gwtsample_mobilewebapp__'] = but that's more a matter of
personal taste)

Oh, and BTW, the page is protected (in web.xml), so there's no need for
the loginUrl here.

http://gwt-code-reviews.appspot.com/1829803/diff/1/samples/mobilewebapp/src/main/webapp/WEB-INF/web.xml
File samples/mobilewebapp/src/main/webapp/WEB-INF/web.xml (right):

http://gwt-code-reviews.appspot.com/1829803/diff/1/samples/mobilewebapp/src/main/webapp/WEB-INF/web.xml#newcode10
samples/mobilewebapp/src/main/webapp/WEB-INF/web.xml:10: auth filters in
place for all other RPC calls. Thre's no real need to protect the
compiled
authentication auth filters

http://gwt-code-reviews.appspot.com/1829803/diff/9003/samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/server/GaeAuthFilter.java
File
samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/server/GaeAuthFilter.java
(right):

http://gwt-code-reviews.appspot.com/1829803/diff/9003/samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/server/GaeAuthFilter.java#newcode93
samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/server/GaeAuthFilter.java:93:
homePageUrl.append(scheme);
No need for all this; UserService accepts paths, so

   / + homePageFileName + (queryString == null ?  : ? +
queryString)

would be enough (even though I'd rather keep only the gwt.codesvr
param from the queryString)

[gwt-contrib] Re: Removed the instance method get(Element element) method from the (issue1831803)

[t . broyer]

My 2 c€nts


http://gwt-code-reviews.appspot.com/1831803/diff/1/user/src/com/google/gwt/aria/client/Roles.java
File user/src/com/google/gwt/aria/client/Roles.java (right):

http://gwt-code-reviews.appspot.com/1831803/diff/1/user/src/com/google/gwt/aria/client/Roles.java#newcode137
user/src/com/google/gwt/aria/client/Roles.java:137: private static
MapString, Role ROLES_MAP = new HashMapString, Role();
Should probably be 'final'.
We might want to use a Collections.unmodifiableMap too (but it's private
so it's not strictly necessary)

http://gwt-code-reviews.appspot.com/1831803/diff/1/user/src/com/google/gwt/aria/client/Roles.java#newcode456
user/src/com/google/gwt/aria/client/Roles.java:456: for (String
testRoleName : roleAttributeValue.split( )) {
How about split(\\s+) instead?

Also, a com.google.gwt.regex.shared.RegExp (initialized as a constant)
might perform better (java.lang.String#split has all sorts of
weirdnesses that GWT has to emulate)

Finally, is there a risk that roleAttributeValue be 'null'?

http://gwt-code-reviews.appspot.com/1831803/diff/1/user/src/com/google/gwt/aria/client/Roles.java#newcode457
user/src/com/google/gwt/aria/client/Roles.java:457: if
(ROLES_MAP.containsKey(testRoleName)) {
How about:
  Role role = ROLES_MAP.get(testRoleName);
  if (role != null) {
return role;
  }

It makes for a single lookup in the map, but maybe this is not necessary
in practice, with no significant performance improvement.

http://gwt-code-reviews.appspot.com/1831803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Add a getter for XHR's responseType. (issue1830803)

[t . broyer]

On 2012/09/13 17:16:52, skybrian wrote:


Since this API is essentially just a wrapper around the JavaScript

object and

there's no place to stash the desired response type, adding the setter

and

deprecating create() with a response type seems like the way to go.


Done.

I wonder if we shouldn't add a public getter for
ResponseType.responseTypeString though, to allow for easier testing of
getResponseType return values:

   ResponseType.ArrayBuffer.whatever.equals(xhr.getResponseType());

For instance, all enums in com.google.gwt.dom.client.Style have a
getCssName().

An alternative would be to add a public boolean
isResponseType(ResponseType responseType) to XMLHttpRequest, but we
don't have anything like that anywhere.

Note: the code itself is still untested.

https://gwt-code-reviews.appspot.com/1830803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Add a getter for XHR's responseType. (issue1830803)

[t . broyer]

https://gwt-code-reviews.appspot.com/1830803/diff/2003/user/src/com/google/gwt/xhr/client/XMLHttpRequest.java
File user/src/com/google/gwt/xhr/client/XMLHttpRequest.java (right):

https://gwt-code-reviews.appspot.com/1830803/diff/2003/user/src/com/google/gwt/xhr/client/XMLHttpRequest.java#newcode150
user/src/com/google/gwt/xhr/client/XMLHttpRequest.java:150: if (xhr !=
null) {
Note: it changes slightly the behavior compared to the previous version.
Previously, xhr.responseType wouldn't be set for ResponseType.Default
(the empty string). There are little to no reasons to pass Default here
though, as its the default behavior of the zero-arg create() factory
method (which used to use Default, so the behavior is totally untouched
there).

https://gwt-code-reviews.appspot.com/1830803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Synchronize static Maps used for caching. (issue1829804)

[t . broyer]

Reviewers: unnurg, jtamplin,

Message:
I've grep'ed in all com.google.web.bindery and didn't find any other
instance.
I also quickly looked into com.google.gwt in case there could have been
similar issues with GWT-RPC or some shared code, but didn't find
anything either.

Description:
Synchronize static Maps used for caching.


Please review this at https://gwt-code-reviews.appspot.com/1829804/

Affected files:
  M  
user/src/com/google/web/bindery/autobean/shared/impl/AutoBeanCodexImpl.java



Index:  
user/src/com/google/web/bindery/autobean/shared/impl/AutoBeanCodexImpl.java
diff --git  
a/user/src/com/google/web/bindery/autobean/shared/impl/AutoBeanCodexImpl.java  
b/user/src/com/google/web/bindery/autobean/shared/impl/AutoBeanCodexImpl.java
index  
78b4f2e4eed6fa6363e9584eb9d76bc54606f11e..5170f2bb208c2923e415e749cb14222f8b35f1ef  
100644
---  
a/user/src/com/google/web/bindery/autobean/shared/impl/AutoBeanCodexImpl.java
+++  
b/user/src/com/google/web/bindery/autobean/shared/impl/AutoBeanCodexImpl.java

@@ -515,16 +515,18 @@ public class AutoBeanCodexImpl {
   }

   public static Coder doCoderFor(AutoBean? bean, String propertyName) {
-String key = key(bean, propertyName);
-Coder toReturn = coderFor.get(key);
-if (toReturn == null) {
-  bean.accept(new PropertyCoderCreator());
-  toReturn = coderFor.get(key);
+synchronized (coderFor) {
+  String key = key(bean, propertyName);
+  Coder toReturn = coderFor.get(key);
   if (toReturn == null) {
-throw new IllegalArgumentException(propertyName);
+bean.accept(new PropertyCoderCreator());
+toReturn = coderFor.get(key);
+if (toReturn == null) {
+  throw new IllegalArgumentException(propertyName);
+}
   }
+  return toReturn;
 }
-return toReturn;
   }

   public static T AutoBeanT doDecode(EncodeState state, ClassT  
clazz, Splittable data) {

@@ -562,12 +564,14 @@ public class AutoBeanCodexImpl {
   }

   public static E extends Enum? Coder enumCoder(ClassE type) {
-Coder toReturn = coders.get(type);
-if (toReturn == null) {
-  toReturn = new EnumCoderE(type);
-  coders.put(type, toReturn);
+synchronized (coders) {
+  Coder toReturn = coders.get(type);
+  if (toReturn == null) {
+toReturn = new EnumCoderE(type);
+coders.put(type, toReturn);
+  }
+  return toReturn;
 }
-return toReturn;
   }

   public static Coder mapCoder(Coder valueCoder, Coder keyCoder) {
@@ -575,12 +579,14 @@ public class AutoBeanCodexImpl {
   }

   public static Coder objectCoder(Class? type) {
-Coder toReturn = coders.get(type);
-if (toReturn == null) {
-  toReturn = new ObjectCoder(type);
-  coders.put(type, toReturn);
+synchronized (coders) {
+  Coder toReturn = coders.get(type);
+  if (toReturn == null) {
+toReturn = new ObjectCoder(type);
+coders.put(type, toReturn);
+  }
+  return toReturn;
 }
-return toReturn;
   }

   public static Coder splittableCoder() {
@@ -588,12 +594,14 @@ public class AutoBeanCodexImpl {
   }

   public static Coder valueCoder(Class? type) {
-Coder toReturn = coders.get(type);
-if (toReturn == null) {
-  toReturn = new ValueCoder(type);
-  coders.put(type, toReturn);
+synchronized (coders) {
+  Coder toReturn = coders.get(type);
+  if (toReturn == null) {
+toReturn = new ValueCoder(type);
+coders.put(type, toReturn);
+  }
+  return toReturn;
 }
-return toReturn;
   }

   static Splittable tryExtractSplittable(Object value) {


--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Issue 7381: 2 RequestFactory with 2 differents Proxy on the same domain class (issue1712803)

[t . broyer]

On 2012/09/13 18:35:35, manolo.carrasco wrote:

We are using domain interfaces in a couple of projects and finally we

decided to

patch this file to avoid such amount of extra code.
I thought this was a very small piece of code which IMHO fixes an

important

issue, and could be related with this commit, so that is the reason to

suggest

the inclusion in 2.5. But if you guys consider that this could wait

for a next

release can wait.


Well, the thing is: Alexandre's issue only happens under some
circumstancies, and the fix only adds an overhead in those cases. The
possible workaroundof tweaking the Deobfuscator.Builder-s to contain the
merged collections is hardly applicable, as there are good chances the
two RequestFactory interfaces come from different JARs, that end up
being deployed in the same WAR.

If it can make you feel better, I have a bunch of patches that are
critical to our app but had to be delayed to the next version too (and
this was decided before RC1)

https://gwt-code-reviews.appspot.com/1712803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Add a getter for XHR's responseType. (issue1830803)

[t . broyer]

On 2012/09/13 18:23:04, skybrian wrote:

I don't see any usages of XmlHttpRequest.ResponseType in Google code.


OK, so I'll remove the create(ResponseType).


We could have constants instead of the enum.


I kind of like the enums as in com.google.gwt.dom.client.Style (also
used in SafeStyles), but I'll make the setResponseType(String) public,
and add a getResponseTypeString() to the enum.

https://gwt-code-reviews.appspot.com/1830803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Synchronize static Maps used for caching. (issue1829804)

[t . broyer]

https://gwt-code-reviews.appspot.com/1829804/diff/1/user/src/com/google/web/bindery/autobean/shared/impl/AutoBeanCodexImpl.java
File
user/src/com/google/web/bindery/autobean/shared/impl/AutoBeanCodexImpl.java
(right):

https://gwt-code-reviews.appspot.com/1829804/diff/1/user/src/com/google/web/bindery/autobean/shared/impl/AutoBeanCodexImpl.java#newcode518
user/src/com/google/web/bindery/autobean/shared/impl/AutoBeanCodexImpl.java:518:
synchronized (coderFor) {
On 2012/09/13 18:48:19, jtamplin wrote:

Isn't it considered bad practice to synchronize on the object you are

protecting

like this?  I don't think it is a problem with HashMap, but I would

still prefer

having explicit coderForLock and codersLock objects.


That's what I thought too, and I initially had the lock objects, but:
 - we already have such synchronization in com.google.gwt.rpc.server.RPC
and com.google.gwt.user.server.rpc.RPC
 - given our use of the map (as a cache, with only get and put), it
shouldn't be a problem.

I don't mind adding the lock objects back though. What do others think?

https://gwt-code-reviews.appspot.com/1829804/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Add a getter for XHR's responseType. (issue1830803)

[t . broyer]

Reviewers: jtamplin, cromwellian,

Message:
Follow-up to https://gwt-code-reviews.appspot.com/1820806

See http://code.google.com/p/google-web-toolkit/issues/detail?id=7386#c8

Description:
Add a getter for XHR's responseType.

Issue 7386


Please review this at https://gwt-code-reviews.appspot.com/1830803/

Affected files:
  M user/src/com/google/gwt/xhr/client/XMLHttpRequest.java


Index: user/src/com/google/gwt/xhr/client/XMLHttpRequest.java
diff --git a/user/src/com/google/gwt/xhr/client/XMLHttpRequest.java  
b/user/src/com/google/gwt/xhr/client/XMLHttpRequest.java
index  
d35d1964cb30c4a3f1c895bdce2db50665f3e32f..aedb80697b26f086391ef305c24a0571c05855bc  
100644

--- a/user/src/com/google/gwt/xhr/client/XMLHttpRequest.java
+++ b/user/src/com/google/gwt/xhr/client/XMLHttpRequest.java
@@ -162,8 +162,8 @@ public class XMLHttpRequest extends JavaScriptObject {
   /**
* Aborts the current request.
* p
-   * See a href=http://www.w3.org/TR/XMLHttpRequest/#abort;
-   * http://www.w3.org/TR/XMLHttpRequest/#abort/a.
+   * See a href=http://www.w3.org/TR/XMLHttpRequest/#the-abort-method;
+   * http://www.w3.org/TR/XMLHttpRequest/#the-abort-method/a.
*/
   public final native void abort() /*-{
 this.abort();
@@ -172,8 +172,8 @@ public class XMLHttpRequest extends JavaScriptObject {
   /**
* Clears the {@link ReadyStateChangeHandler}.
* p
-   * See a href=http://www.w3.org/TR/XMLHttpRequest/#onreadystatechange;
-   * http://www.w3.org/TR/XMLHttpRequest/#onreadystatechange/a.
+   * See a  
href=http://www.w3.org/TR/XMLHttpRequest/#handler-xhr-onreadystatechange;
+   *  

http://www.w3.org/TR/XMLHttpRequest/#handler-xhr-onreadystatechange/a.

*
* @see #clearOnReadyStateChange()
*/
@@ -189,8 +189,8 @@ public class XMLHttpRequest extends JavaScriptObject {
   /**
* Gets all the HTTP response headers, as a single string.
* p
-   * See a  
href=http://www.w3.org/TR/XMLHttpRequest/#getallresponseheaders;

-   * http://www.w3.org/TR/XMLHttpRequest/#getallresponseheaders/a.
+   * See a  
href=http://www.w3.org/TR/XMLHttpRequest/#the-getallresponseheaders-method;
+   *  

http://www.w3.org/TR/XMLHttpRequest/#the-getallresponseheaders-method/a.

*
* @return the response headers.
*/
@@ -201,8 +201,8 @@ public class XMLHttpRequest extends JavaScriptObject {
   /**
* Get's the current ready-state.
* p
-   * See a href=http://www.w3.org/TR/XMLHttpRequest/#readystate;
-   * http://www.w3.org/TR/XMLHttpRequest/#readystate/a.
+   * See a  
href=http://www.w3.org/TR/XMLHttpRequest/#dom-xmlhttprequest-readystate;

+   * http://www.w3.org/TR/XMLHttpRequest/#dom-xmlhttprequest-state/a.
*
* @return the ready-state constant
*/
@@ -223,8 +223,8 @@ public class XMLHttpRequest extends JavaScriptObject {
   /**
* Gets an HTTP response header.
* p
-   * See a href=http://www.w3.org/TR/XMLHttpRequest/#getresponseheader;
-   * http://www.w3.org/TR/XMLHttpRequest/#getresponseheader/a.
+   * See a  
href=http://www.w3.org/TR/XMLHttpRequest/#the-getresponseheader-method;
+   *  

http://www.w3.org/TR/XMLHttpRequest/#the-getresponseheader-method/a.

*
* @param header the response header to be retrieved
* @return the header value
@@ -236,8 +236,8 @@ public class XMLHttpRequest extends JavaScriptObject {
   /**
* Gets the response text.
* p
-   * See a href=http://www.w3.org/TR/XMLHttpRequest/#responsetext;
-   * http://www.w3.org/TR/XMLHttpRequest/#responsetext/a.
+   * See a  
href=http://www.w3.org/TR/XMLHttpRequest/#the-responsetext-attribute;

+   * http://www.w3.org/TR/XMLHttpRequest/#the-responsetext-attribute/a.
*
* @return the response text
*/
@@ -246,10 +246,22 @@ public class XMLHttpRequest extends JavaScriptObject {
   }-*/;

   /**
+   * Gets the response type.
+   * p
+   * See a  
href=http://www.w3.org/TR/XMLHttpRequest/#the-responsetype-attribute;

+   * http://www.w3.org/TR/XMLHttpRequest/#the-responsetype-attribute/a
+   *
+   * @return the response type
+   */
+  public final native String getResponseType() /*-{
+return this.responseType || ;
+  }-*/;
+
+  /**
* Gets the status code.
* p
-   * See a href=http://www.w3.org/TR/XMLHttpRequest/#status;
-   * http://www.w3.org/TR/XMLHttpRequest/#status/a.
+   * See a  
href=http://www.w3.org/TR/XMLHttpRequest/#the-status-attribute;

+   * http://www.w3.org/TR/XMLHttpRequest/#the-status-attribute/a.
*
* @return the status code
*/
@@ -260,8 +272,8 @@ public class XMLHttpRequest extends JavaScriptObject {
   /**
* Gets the status text.
* p
-   * See a href=http://www.w3.org/TR/XMLHttpRequest/#statustext;
-   * http://www.w3.org/TR/XMLHttpRequest/#statustext/a.
+   * See a  
href=http://www.w3.org/TR/XMLHttpRequest/#the-statustext-attribute;

+   * http://www.w3.org/TR/XMLHttpRequest/#the-statustext-attribute/a.
*
* @return the status text
*/
@@ -272,8 +284,8 @@ public class XMLHttpRequest extends JavaScriptObject {
   /**
* 

[gwt-contrib] Re: Update Maven sample pom.xml files to use maven-compiler-plugin's annotation processing functiona... (issue1828803)

[t . broyer]

http://gwt-code-reviews.appspot.com/1828803/diff/1/samples/dynatablerf/pom.xml
File samples/dynatablerf/pom.xml (right):

http://gwt-code-reviews.appspot.com/1828803/diff/1/samples/dynatablerf/pom.xml#newcode55
samples/dynatablerf/pom.xml:55: !-- Doesn't yet work in eclipse. See
maven-processor-plugin below.
This dependency is no longer needed (and the comment wrong)

http://gwt-code-reviews.appspot.com/1828803/diff/1/samples/dynatablerf/pom.xml#newcode110
samples/dynatablerf/pom.xml:110: source1.6/source
Those tabs should be spaces.

http://gwt-code-reviews.appspot.com/1828803/diff/1/samples/dynatablerf/pom.xml#newcode128
samples/dynatablerf/pom.xml:128: version2.3.0-1/version
Latest version is 2.5.0-rc1, which depends on GWT 2.5.0-rc1 (so the
plugin dependencies below are no longer needed; FYI, it also removes the
unneeded dependency on gwt-servlet)

FYI, I'll publish 2.5.0-rc2 as soon as GWT 2.5 RC2 is out, and similarly
for the final release.

All in all, I'd keep the plugin dependencies in (minus gwt-servlet), as
it's unlikely gwt-maven-plugin 2.5.0-rc2 is released on the same week as
GWT 2.5 RC2, making it impossible to bump gwtVersion in time for the GWT
release (there would be a version mismatch between the project
dependencies and the plugin dependencies) or making the samples unusable
for a few days (in case both versions are changed, in prevision to the
gwt-maven-plugin release).

In the long term, I'd like the gwt-maven-plugin to be adopted
(rewritten?) by the GWT project, but that's another story ;-)

http://gwt-code-reviews.appspot.com/1828803/diff/1/samples/dynatablerf/pom.xml#newcode193
samples/dynatablerf/pom.xml:193: !-- Mark the project for Google Plugin
for Eclipse (GPE) --
Is this still needed? Isn't the GPE smart enough?
(I thought it'd automatically add the nature and build command upon
detecting the use of the gwt-maven-plugin and/or gwt-user dependency)

http://gwt-code-reviews.appspot.com/1828803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Add a getter for XHR's responseType. (issue1830803)

[t . broyer]

On 2012/09/12 14:32:37, jtamplin wrote:

Did you test it?  It probably needs testing on IE6 if GWT still

officially

supports it, since I recall some weirdness about getting exceptions if

you

referenced properties that didn't exist on native objects that were

exposed to

JS.


Just ran the following http://jsfiddle.net/m3Saz/ in IE6.
It prints undefined on getting and throws on setting.

BTW, in this test, both Firefox 15 and Aurora (16) are failing when
setting to arraybuffer: Error setting to 'arraybuffer': [Exception...
An attempt was made to use an object that is not, or is no longer,
usable code: 11 nsresult: 0x8053000b (InvalidStateError) location:
http://fiddle.jshell.net/m3Saz/show/ Line: 45] It's works when
setting to dummy though (and returns the empty string).

It works if I call xhr.open() before trying to set the responseType,
which means the XMLHttpRequest API we chose (create(ResponseType)) is
not going to work in Firefox (this is a bug in Firefox though, the spec
says it should work, but still). So maybe we need a setter instead of
(or in addition to) an argument create() ?

Chrome (23-dev) is OK (printing the empty string, then arraybuffer and
arraybuffer)

I'll try on IE7, IE8 and IE9 next (currently downloading the VMs from
the MSDN).

https://gwt-code-reviews.appspot.com/1830803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Add a getter for XHR's responseType. (issue1830803)

[t . broyer]

On 2012/09/12 16:03:13, tbroyer wrote:

I'll try on IE7, IE8 and IE9 next (currently downloading the VMs from

the MSDN).

Results from https://browserlab.adobe.com

IE7, IE8 and IE9: works as an expando (no error; prints undefined,
arraybuffer, dummy)
Chrome 18 / Windows: throws on setting to dummy, otherwise OK
Safari 5.1 / OSX: expando (undefined, arraybuffer, dummy)
Firefox 11 (Windows / OSX): throws on setting (probably the same bug as
in FF15 and Aurora but they also throw when setting to dummy, haven't
tried with open() though)

https://gwt-code-reviews.appspot.com/1830803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: - Extended the a11y role API with methods that have UIObject parameter. (issue1828804)

[t . broyer]

I haven't looked at the changes (from the description, they should be
simple and straightforward) but what bothers me is that it introduces a
cyclic dependency: c.g.g.user depends on c.g.g.aria which now also
depends on c.g.g.user.

Also, that dependency isn't reflected in the .gwt.xml (so one could say
it's not really a cyclic dependency, but then you'll have source code
not found for UIObject if you don't use c.g.g.user in your app).

I think we should rather introduce an HasElement interface (e.g. in
c.g.g.dom) that UIObject would implement.

http://gwt-code-reviews.appspot.com/1828804/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: - Extended the a11y role API with methods that have UIObject parameter. (issue1828804)

[t . broyer]

On 2012/09/12 19:18:12, skybrian wrote:

We've decided against this change for now (increases the size of the

API too

much). A HasElement interface sounds somewhat reasonable but we can

add that

later if needed.



I suppose if Element implemented HasElement, returning itself, we

could avoid

doubling the number of methods, but this seems overly tricky when

calling

getElement() yourself isn't hard.


Plus: some widgets already have ARIA roles.
So maybe a better long-term solution would be to add methods on
generic widgets to allow one to set a role for it (e.g.
setWidgetRole(Widget, Role) on DockLayoutPanel, setRole(Role) on
SimplePanel, etc.), so that those widgets with already-specified roles
(e.g. Tree/CellTree, CellTable, TabPanel, etc.) cannot be given a new
role without some trickery (at the very least a call to getElement(),
which is a generally-understood signal that from now one, if something
break, blame yourself).

http://gwt-code-reviews.appspot.com/1828804/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: EditorDriver#setConstraintViolations used to thrown NPE if arg was null. (issue1826803)

[t . broyer]

On 2012/09/10 22:57:14, skybrian wrote:

I don't know this code, but since pushViolations() and
iterableFromConstraintViolations() are both public methods, under the

principle

of being conservative about what you send and liberal about what you

receive, it

seems like iterableFromConstraintViolations() should return an empty

iterable

when given a null (since whatever calls it probably expects to be able

to

iterate) and pushViolations() should do the null check.


OK, I moved the ternary to BaseEditorDriver which will either pass a
null directly to pushViolations or will pass a non-null value to
iterableFromConstrantViolations.

I haven't tried to fix iterableFromConstrantViolations: if you pass a
null, you'll have an NPE as soon as your try to use the returned
iterable.

Note: I've verified that this is the only use of
iterableFromConstrantViolations.

https://gwt-code-reviews.appspot.com/1826803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Introduce SuggestBoxBaseT extends ValueBoxBaseString (issue 6494102)

[t . broyer]

I think I prefer the other option about changing SuggestBox.


https://codereview.appspot.com/6494102/diff/1/user/src/com/google/gwt/user/client/ui/SuggestBox.java
File user/src/com/google/gwt/user/client/ui/SuggestBox.java (right):

https://codereview.appspot.com/6494102/diff/1/user/src/com/google/gwt/user/client/ui/SuggestBox.java#newcode104
user/src/com/google/gwt/user/client/ui/SuggestBox.java:104: * @see
ValueBoxBase
Why change this line? (I suppose it's a leftover of the other proposed
patch)

https://codereview.appspot.com/6494102/diff/1/user/src/com/google/gwt/user/client/ui/SuggestBox.java#newcode107
user/src/com/google/gwt/user/client/ui/SuggestBox.java:107: public class
SuggestBox extends SuggestBoxBaseTextBox {
Should be TextBoxBase here, not TextBox.

https://codereview.appspot.com/6494102/diff/1/user/src/com/google/gwt/user/client/ui/SuggestBoxBase.java
File user/src/com/google/gwt/user/client/ui/SuggestBoxBase.java (right):

https://codereview.appspot.com/6494102/diff/1/user/src/com/google/gwt/user/client/ui/SuggestBoxBase.java#newcode50
user/src/com/google/gwt/user/client/ui/SuggestBoxBase.java:50: public
class SuggestBoxBaseT extends ValueBoxBaseString extends Composite
implements HasText, HasFocus,
Javadoc

https://codereview.appspot.com/6494102/diff/1/user/src/com/google/gwt/user/client/ui/SuggestBoxBase.java#newcode792
user/src/com/google/gwt/user/client/ui/SuggestBoxBase.java:792: public
void hideSuggestionList() {
All those deprecated methods that have been moved to
DefaultSuggestionDisplay should probably stay in SuggestBox.

https://codereview.appspot.com/6494102/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Breaking Change: Use ValueBoxBaseString instead of TextBox in SuggestBox (issue 6492092)

[t . broyer]

My preferred option, with one small adjustment to make it a non-breaking
change.


https://codereview.appspot.com/6492092/diff/1/user/src/com/google/gwt/user/client/ui/SuggestBox.java
File user/src/com/google/gwt/user/client/ui/SuggestBox.java (right):

https://codereview.appspot.com/6492092/diff/1/user/src/com/google/gwt/user/client/ui/SuggestBox.java#newcode877
user/src/com/google/gwt/user/client/ui/SuggestBox.java:877: public
ValueBoxBaseString getTextBox() {
How about adding a getValueBox() instead, and let this method throw a
ClassCastException in case 'box' is not a TextBoxBase?
(and deprecate it in favor of getValueBox)

That way, we wouldn't even break anyone's code.

https://codereview.appspot.com/6492092/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Breaking Change: Use ValueBoxBaseString instead of TextBox in SuggestBox (issue 6492092)

[t . broyer]

On 2012/09/10 12:03:20, jtamplin wrote:

BTW: you should create code reviews at

http://gwt-code-reviews.appspot.com/

instead.



There is a also a TODO to support SafeHtml here.  I'm not entirely

sure what

that means in this case (Doesn't TextBox already ensure the string is
uninterpreted?  Does changing to a ValueBoxBase mean you might be

displaying

HTML in a value?), but we should either make sure this change moves us

closer to

that or remove the TODO.


Could it have rather been about the SuggestionDisplay?
Sounds hard to add (would require a breaking-change in
SuggestOracle.Suggestion, or possibly a new interface, and a breaking
change to SuggestBox.SuggestionDisplay).
At this point, we'd probably better rebuild SuggestBox from scratch with
better modularity (possibly even build it as an extender/behavior,
similar to ASP.NET AJAX's AutoComplete
http://www.asp.net/ajaxlibrary/act_AutoComplete.ashx) and allowing for
auto-completing parts of the valuebox's value (similar to Closure
Library's AutoComplete
http://closure-library.googlecode.com/svn/docs/class_goog_ui_ac_AutoComplete.html)

In other words, we should probably remove this TODO, or change it to a
comment suggesting the above.

https://codereview.appspot.com/6492092/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Breaking Change: Use ValueBoxBaseString instead of TextBox in SuggestBox (issue 6492092)

[t . broyer]

LGTM


https://codereview.appspot.com/6492092/diff/5001/user/src/com/google/gwt/user/client/ui/SuggestBox.java
File user/src/com/google/gwt/user/client/ui/SuggestBox.java (right):

https://codereview.appspot.com/6492092/diff/5001/user/src/com/google/gwt/user/client/ui/SuggestBox.java#newcode878
user/src/com/google/gwt/user/client/ui/SuggestBox.java:878: *
@deprecated in favour of getValueBox
On 2012/09/10 15:46:14, jtamplin wrote:

If there is going to be an official policy of removing
deprecated items,


I really hope so!
Proposed it a couple times already but received no feedback so far :-(


should list a date or a release version where it will be
removed here?


In the mean time, maybe list the first release version where it'll be
marked deprecated (i.e. 2.6 here).

https://codereview.appspot.com/6492092/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] CellTree disappeared when clicking in the widget but outside tree nodes. (issue1827803)

[t . broyer]

Reviewers: unnurg,

Description:
CellTree disappeared when clicking in the widget but outside tree nodes.

Issue 6359


Please review this at https://gwt-code-reviews.appspot.com/1827803/

Affected files:
  M user/src/com/google/gwt/user/cellview/client/CellTree.java


Index: user/src/com/google/gwt/user/cellview/client/CellTree.java
diff --git a/user/src/com/google/gwt/user/cellview/client/CellTree.java  
b/user/src/com/google/gwt/user/cellview/client/CellTree.java
index  
7b12a0a5ba6ddfa0c5cb882dc91443ecb11d5167..4847ee63f45b24e983c2ab7856332ad3f1dce138  
100644

--- a/user/src/com/google/gwt/user/cellview/client/CellTree.java
+++ b/user/src/com/google/gwt/user/cellview/client/CellTree.java
@@ -740,7 +740,7 @@ public class CellTree extends AbstractCellTree  
implements HasAnimation,

 if (nodeView != null) {
   if (isMouseDown) {
 Element showMoreElem = nodeView.getShowMoreElement();
-if (nodeView.getImageElement().isOrHasChild(target)) {
+if (!nodeView.isRootNode()   
nodeView.getImageElement().isOrHasChild(target)) {

   // Open the node when the open image is clicked.
   nodeView.setOpen(!nodeView.isOpen(), true);
   return;


--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Add aria-hidden state to layout panel rulers. (issue1820807)

[t . broyer]

Reviewers: atincheva,

Message:
Note: I only applied the suggested fix (after checking the WAI-ARIA
spec), I haven't tried it with a screenreader (actually, it's so
straightforward I haven't tried it at all).

Description:
Add aria-hidden state to layout panel rulers.

Issue 7646


Please review this at https://gwt-code-reviews.appspot.com/1820807/

Affected files:
  M user/src/com/google/gwt/layout/Layout.gwt.xml
  M user/src/com/google/gwt/layout/client/LayoutImpl.java


Index: user/src/com/google/gwt/layout/Layout.gwt.xml
diff --git a/user/src/com/google/gwt/layout/Layout.gwt.xml  
b/user/src/com/google/gwt/layout/Layout.gwt.xml
index  
65797b57adfe4da195bc3e2a0fd5887a6134e7a4..c7a22f55c8ddb0e37b0c4bca3347b63c5aa0b721  
100644

--- a/user/src/com/google/gwt/layout/Layout.gwt.xml
+++ b/user/src/com/google/gwt/layout/Layout.gwt.xml
@@ -14,9 +14,12 @@

 module
   inherits name=com.google.gwt.core.Core/
-  inherits name=com.google.gwt.user.UserAgent/
+  inherits name=com.google.gwt.useragent.UserAgent/
   inherits name=com.google.gwt.dom.DOM/
   inherits name=com.google.gwt.animation.Animation/
+  inherits name=com.google.gwt.aria.Aria/
+  !--  LayoutImplIE6 uses c.g.g.u.c.Window --
+  inherits name=com.google.gwt.user.Window/

   replace-with class=com.google.gwt.layout.client.LayoutImplIE8
 when-type-is class=com.google.gwt.layout.client.LayoutImpl/
Index: user/src/com/google/gwt/layout/client/LayoutImpl.java
diff --git a/user/src/com/google/gwt/layout/client/LayoutImpl.java  
b/user/src/com/google/gwt/layout/client/LayoutImpl.java
index  
a21f3f73914498922d1e32a4a8ab4301519516f8..98163336d59009ced735df50466aca897c19c093  
100644

--- a/user/src/com/google/gwt/layout/client/LayoutImpl.java
+++ b/user/src/com/google/gwt/layout/client/LayoutImpl.java
@@ -19,6 +19,7 @@ import static com.google.gwt.dom.client.Style.Unit.EM;
 import static com.google.gwt.dom.client.Style.Unit.EX;
 import static com.google.gwt.dom.client.Style.Unit.PX;

+import com.google.gwt.aria.client.State;
 import com.google.gwt.dom.client.DivElement;
 import com.google.gwt.dom.client.Document;
 import com.google.gwt.dom.client.Element;
@@ -62,6 +63,8 @@ class LayoutImpl {
 // extra precision.
 style.setWidth(10, widthUnit);
 style.setHeight(10, heightUnit);
+
+State.HIDDEN.set(ruler, true);
 return ruler;
   }



--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Add aria-hidden state to layout panel rulers. (issue1820807)

[t . broyer]

Adding Brian as reviewer, as discussed with Unnur.

https://gwt-code-reviews.appspot.com/1820807/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: CellTree disappeared when clicking in the widget but outside tree nodes. (issue1827803)

[t . broyer]

Brian: Unnur thinks you'd review that patch faster than her ;-)


https://gwt-code-reviews.appspot.com/1827803/diff/1/user/src/com/google/gwt/user/cellview/client/CellTree.java
File user/src/com/google/gwt/user/cellview/client/CellTree.java (right):

https://gwt-code-reviews.appspot.com/1827803/diff/1/user/src/com/google/gwt/user/cellview/client/CellTree.java#newcode743
user/src/com/google/gwt/user/cellview/client/CellTree.java:743: if
(!nodeView.isRootNode() 
nodeView.getImageElement().isOrHasChild(target)) {
On 2012/09/10 23:02:41, unnurg wrote:

Does the root node never need to be opened?  Or does setNode() only

add that

CSS that we're trying to get rid of?


Actually, the CSS is not the issue, it's only what triggers it: the
actual issue is that the root node is closed when receiving a mousedown
event; the margin:20px makes it so that there are 20px high regions
between top-level nodes where clicks target the root node.
And the real issue is that getImageElement accesses some specific
element in the DOM, assuming the node's element contains a first child
for the node's value and disclosure image (the one that getImageElement
returns) and a second one with the children. Except that for the root
node, there's no visible value (it's a virtual node) so the second
child is actually the first, and getImageElement returns a DIV that
contains all children (so when clicking in between child nodes, it
targets getImageElement, producing a false positive here).

Yes, it's a bit of a complex thing ;-)

The root node is opened in the constructor and should never ever be
closed (remember: it's invisible, it's only there to contain the
top-level nodes).

https://gwt-code-reviews.appspot.com/1827803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] xhr.responseType was misspelled as responsetype (lowercase t) (issue1820806)

[t . broyer]

Reviewers: jtamplin, unnurg,

Description:
xhr.responseType was misspelled as responsetype (lowercase t)

Issue 7386


Please review this at https://gwt-code-reviews.appspot.com/1820806/

Affected files:
  M user/src/com/google/gwt/xhr/client/XMLHttpRequest.java


Index: user/src/com/google/gwt/xhr/client/XMLHttpRequest.java
diff --git a/user/src/com/google/gwt/xhr/client/XMLHttpRequest.java  
b/user/src/com/google/gwt/xhr/client/XMLHttpRequest.java
index  
925c33ad1d871e8b7bb3122e60b4104b437da8bd..d35d1964cb30c4a3f1c895bdce2db50665f3e32f  
100644

--- a/user/src/com/google/gwt/xhr/client/XMLHttpRequest.java
+++ b/user/src/com/google/gwt/xhr/client/XMLHttpRequest.java
@@ -151,7 +151,7 @@ public class XMLHttpRequest extends JavaScriptObject {
   }
 }
 if (xhr  responseType) {
-  xhr.responsetype = responseType;
+  xhr.responseType = responseType;
 }
 return xhr;
   }-*/;


--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: xhr.responseType was misspelled as responsetype (lowercase t) (issue1820806)

[t . broyer]

On 2012/09/09 23:24:59, jtamplin wrote:


We really need to get an updated HtmlUnit so we can have automated

tests for

some of these things.


I don't know the status of HtmlUnit, but trying new versions should be
made much easier with the move Maven. That being said, I skimmed
HtmlUnit's Web site and it doesn't seem to support either TypedArrays or
xhr.responseType :-(

https://gwt-code-reviews.appspot.com/1820806/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] EditorDriver#setConstraintViolations used to thrown NPE if arg was null. (issue1826803)

[t . broyer]

Reviewers: skybrian,

Description:
EditorDriver#setConstraintViolations used to thrown NPE if arg was null.

Issue 6578


Please review this at https://gwt-code-reviews.appspot.com/1826803/

Affected files:
  M user/src/com/google/gwt/editor/client/impl/SimpleViolation.java
  M  
user/src/com/google/web/bindery/requestfactory/gwt/client/impl/AbstractRequestFactoryEditorDriver.java

  M user/test/com/google/gwt/editor/client/EditorErrorTest.java


Index: user/src/com/google/gwt/editor/client/impl/SimpleViolation.java
diff --git  
a/user/src/com/google/gwt/editor/client/impl/SimpleViolation.java  
b/user/src/com/google/gwt/editor/client/impl/SimpleViolation.java
index  
ab4e9eaec1dbf30e08b8e81ff1fbc3fbb0665de4..c030aeb57670dc5400fb608bd5404f927148d081  
100644

--- a/user/src/com/google/gwt/editor/client/impl/SimpleViolation.java
+++ b/user/src/com/google/gwt/editor/client/impl/SimpleViolation.java
@@ -100,7 +100,7 @@ public abstract class SimpleViolation {

   public static IterableSimpleViolation iterableFromConstrantViolations(
   IterableConstraintViolation? violations) {
-return new ConstraintViolationIterable(violations);
+return violations == null ? null : new  
ConstraintViolationIterable(violations);

   }

   /**
@@ -109,6 +109,10 @@ public abstract class SimpleViolation {
*/
   public static void pushViolations(IterableSimpleViolation violations,
   EditorDriver? driver, KeyMethod keyMethod) {
+if (violations == null) {
+  return;
+}
+
 DelegateMap delegateMap = DelegateMap.of(driver, keyMethod);

 // For each violation
Index:  
user/src/com/google/web/bindery/requestfactory/gwt/client/impl/AbstractRequestFactoryEditorDriver.java
diff --git  
a/user/src/com/google/web/bindery/requestfactory/gwt/client/impl/AbstractRequestFactoryEditorDriver.java  
b/user/src/com/google/web/bindery/requestfactory/gwt/client/impl/AbstractRequestFactoryEditorDriver.java
index  
fa7aa35e7956504ad9613408c4fb4b7fc36f87b1..330ec434ab571df1abee4ec5b3ae0bf19b4f947f  
100644
---  
a/user/src/com/google/web/bindery/requestfactory/gwt/client/impl/AbstractRequestFactoryEditorDriver.java
+++  
b/user/src/com/google/web/bindery/requestfactory/gwt/client/impl/AbstractRequestFactoryEditorDriver.java
@@ -196,7 +196,7 @@ public abstract class  
AbstractRequestFactoryEditorDriverR, E extends EditorR

   @SuppressWarnings(deprecation)
   public boolean setViolations(
   Iterablecom.google.web.bindery.requestfactory.shared.Violation  
violations) {

-return doSetViolations(new ViolationIterable(violations));
+return doSetViolations(violations == null ? null : new  
ViolationIterable(violations));

   }

   protected void checkSaveRequest() {
Index: user/test/com/google/gwt/editor/client/EditorErrorTest.java
diff --git a/user/test/com/google/gwt/editor/client/EditorErrorTest.java  
b/user/test/com/google/gwt/editor/client/EditorErrorTest.java
index  
8900d5f9c236c38d1f08bb5a1bb7333a2c3c25c5..755ebef191ad74660cd706f321eda01b19395f4b  
100644

--- a/user/test/com/google/gwt/editor/client/EditorErrorTest.java
+++ b/user/test/com/google/gwt/editor/client/EditorErrorTest.java
@@ -23,6 +23,7 @@ import  
com.google.gwt.validation.client.impl.ConstraintViolationImpl;


 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;

@@ -193,6 +194,22 @@ public class EditorErrorTest extends GWTTestCase {
 driver.edit(p);
 driver.flush();
 assertEquals(0, editor.errors.size());
+assertFalse(driver.hasErrors());
+assertNotNull(driver.getErrors());
+assertEquals(0, driver.getErrors().size());
+
+ 
assertFalse(driver.setConstraintViolations(Collections.ConstraintViolation?emptyList()));

+assertEquals(0, editor.errors.size());
+assertFalse(driver.hasErrors());
+assertNotNull(driver.getErrors());
+assertEquals(0, driver.getErrors().size());
+
+// Test no NPE is thrown; see issue 6578
+assertFalse(driver.setConstraintViolations(null));
+assertEquals(0, editor.errors.size());
+assertFalse(driver.hasErrors());
+assertNotNull(driver.getErrors());
+assertEquals(0, driver.getErrors().size());
   }

   public void testSimpleError() {


--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: EditorDriver#setConstraintViolations used to thrown NPE if arg was null. (issue1826803)

[t . broyer]

https://gwt-code-reviews.appspot.com/1826803/diff/2001/user/src/com/google/web/bindery/requestfactory/gwt/client/impl/AbstractRequestFactoryEditorDriver.java
File
user/src/com/google/web/bindery/requestfactory/gwt/client/impl/AbstractRequestFactoryEditorDriver.java
(right):

https://gwt-code-reviews.appspot.com/1826803/diff/2001/user/src/com/google/web/bindery/requestfactory/gwt/client/impl/AbstractRequestFactoryEditorDriver.java#newcode199
user/src/com/google/web/bindery/requestfactory/gwt/client/impl/AbstractRequestFactoryEditorDriver.java:199:
return doSetViolations(violations == null ? null : new
ViolationIterable(violations));
On 2012/09/10 00:21:06, jtamplin wrote:

Wouldn't it be much cleaner elsewhere to just set violations to an

empty list

here if it is null?


It wouldn't be enough.

There's also setConstraintViolations (which replaces this setViolations
–deprecated– method), defined in BaseEditorDriver and which calls
SimpleViolation.iterableFromConstraintViolations.
So, both setViolations here and iterableFromConstraintViolations would
use an empty list here, but that would only save 3 lines in
SimpleViolation.pushViolations.

https://gwt-code-reviews.appspot.com/1826803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

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

[t . broyer]

ping!

(would be great to have it in 2.5)

https://gwt-code-reviews.appspot.com/1772803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Make DateBox implement HasEnabled. (issue1823803)

[t . broyer]

Note this fixes
http://code.google.com/p/google-web-toolkit/issues/detail?id=6197 and
there was already a patch pending review, submitted almost one year ago:
http://gwt-code-reviews.appspot.com/1568803/

http://gwt-code-reviews.appspot.com/1823803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: Not able to use Celltable for a requirement where I need different widgets under one column.

[Adam T]

Hi!

You should be able to use custom table rendering of a GWT 2.5 cell table to 
do what you want in your point (1) (extending AbstractCellTableBuilder and 
implement the buildStandardRowImpl method to put the relevant cell type 
into the column that alters based upon your condition in your point (2). 
 Then pass that renderer to the cell table, instead of the standard one, 
using its setTableBuilder method)

Information about this is a little sparse, but you could check out:

* http://showcase2.jlabanca-testing.appspot.com/#!CwCustomDataGrid , or
* GWTinAction edition 2 http://www.manning.com/tacy - a relevant example 
http://code.google.com/p/gwtinaction2/wiki/DataPresentation(different 
cells in the Tags column) can be found in code 
herehttp://code.google.com/p/gwtinaction2/source/browse/trunk/gwtia-ch10-data-presentation-widgets/src/com/manning/gwtia/ch10/client/datagrid/CellDataGridExample.java.
 
(it is still work in progress just now, so missing comments, but should be 
relatively self explanatory)

Hope that helps in some way!

//Adam

On Sunday, September 2, 2012 11:43:43 AM UTC+2, Saurabh Tripathi wrote:

 Hi all,

 I am stuck here with a requirement which is mentioned as follow:

 1)A table where one or more (for now just one) column need to have 
 different widgets in editable cell.
for example: Class is Plant having property: 'name' and 
 access method: 'String getName()';
   Now if(name.equals(true) || 
 name.equals(false){ 
  --- Render 
 CheckBox with respective checked value
  }else if(name.equals(~)) {
- Render 
 ListBox with some predefined items
  }else {
 -just render 
 the String.
  }
 2)The cell where we render just the String should be Editable (i.e on 
 click textbox appear for edit), other than this other cells should be 
 non-editable i.e cells for checkbox and listbox should be non editable, 

 You may find this requirement very unusual and I agree it is because here 
 we dealing with raw xml/text kind of metadata.
 Anyway I have given a lot of effort over this, but I am in a catch 22 
 situation if somehow I am able to attain 1 condition than I am have 
 editable problem, if somehow I make that up, the valueupdater doesn't seem 
 to work properly

 At last I am thinking that may be 'CellTable' API was never there for such 
 kind of requirement. So FlexTable could be the answer.
 Even if it is so there would be a lot of work for me then like - 
 implementing sorting(while click on header), pagination etc, it would be 
 great if 
 some open source library is already there for it.  

 Thanks in advance 


-- 
You received this message because you are subscribed to the Google Groups 
Google Web Toolkit group.
To view this discussion on the web visit 
https://groups.google.com/d/msg/google-web-toolkit/-/Xy3F9oc6y0EJ.
To post to this group, send email to google-web-toolkit@googlegroups.com.
To unsubscribe from this group, send email to 
google-web-toolkit+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/google-web-toolkit?hl=en.

[gwt-contrib] fix Nullpointer in CellWidget.onBrowserEvent (issue1820805)

[t . broyer]

LGTM

http://gwt-code-reviews.appspot.com/1820805/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Issue 7381: 2 RequestFactory with 2 differents Proxy on the same domain class (issue1712803)

[t . broyer]

https://gwt-code-reviews.appspot.com/1712803/diff/2001/user/src/com/google/web/bindery/requestfactory/vm/impl/Deobfuscator.java
File
user/src/com/google/web/bindery/requestfactory/vm/impl/Deobfuscator.java
(right):

https://gwt-code-reviews.appspot.com/1712803/diff/2001/user/src/com/google/web/bindery/requestfactory/vm/impl/Deobfuscator.java#newcode122
user/src/com/google/web/bindery/requestfactory/vm/impl/Deobfuscator.java:122:
private MapString, ListString merge(MapString, ListString
domainToClientType1,
On 2012/09/06 19:21:57, skybrian wrote:


Hmm. Thanks, but I'm still rather lost and need some context, probably

because I

don't understand the RequestFactory wire protocol.


I'll try to shed some light (though ti has nothing to do with the wire
protocol actually).


To be specific, what is a domain?


Domain types are the kind of objects pointed to by
@ProxyFor/@ProxyForName annotations on proxies (which are here called
client types)


What's an example key to this map?


In the MobileWebApp sample, that would be
com.google.gwt.sample.mobilewebapp.server.domain.Task (a domain type's
binary name).


What is the value?


The list of client types mapped to that domain type, ordered from
the most to the least specific. That is, the proxies with a @ProxyFor or
@ProxyForName pointing to that domain type, and all their
super-interfaces.
In the MobileWebApp sample, for the Task domain type, the values would
be (in order) com.google.gwt.sample.mobilewebapp.shared.TaskProxy and
com.google.web.bindery.requestfactory.shared.EntityProxy.
But you can map several proxies to the same domain type, in which case
you'd have more than that.


Why would we have multiple values for the same key?
What is this map for, anyway?


The map is used when constructing the response.



Let's say I have an Employee and Manager (extends Employee) domain
types, and an EmployeeProxy with @ProxyFor(Employee.class). Some method
(a service method, or a getter in another bean) has a return type of
Employee on the server-side (mapped as EntityProxy on the client-side).
The value could very-well be a Manager in practice.

This is where the map comes into play: which client type should be used?

RF will take the returned value's class (say, Manager) and looks for a
corresponding proxy type that extends EmployeeProxy (because the client
method's return type is EmployeeProxy).
First case: there's no entry with Manager as key, so RF looks at the
superclass. It finds an entry for Employee, with values EmployeeProxy
and EntityProxy. EmployeeProxy matches, so let's use that.
Second case: there's a ManagerProxy extends EmployeeProxy with
@ProxyFor(Manager.class), so RF will find an entry with values
ManagerProxy, EmployeeProxy and EntityProxy. ManagerProxy extends
EmployeeProxy (the type we look for), so let's use that: the client will
receive/reconstruct a ManagerProxy.
Third case: there's a ManagerProxy with no relationship to
EmployeeProxy, and with @ProxyFor(Manager.class). RF will find an entry
for Manager with values ManagerProxy and EntityProxy. Nothing matches
EmployeeProxy, so let's look at the superclass. There's an entry for
Employee with an exact match (EmployeeProxy).

There's also a fourth case: you have some heavy-weight domain object
with many properties, so you map it with 2 proxies: one for all the
properties, and a lightweight one with only a few properties (used for
lists of such proxies, e.g. in search results or in a value picker).
Let's say I have a Message domain type (with subject, date, recipient,
sender and body properties), and I map it with 2 proxies: MessageProxy
with all same properties and MessageProxyLite mapping only subject, date
and sender (with no inheritance relationship between both proxies).
The entry in the map for Message will have values MessageProxy,
MessageProxyLite and EntityProxy.

And of course, all those cases can be mixed and matched.

https://gwt-code-reviews.appspot.com/1712803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Remove long-deprecated event listeners and EventPreview. (issue1822803)

[t . broyer]

Reviewers: cromwellian, rdayal,

Description:
Remove long-deprecated event listeners and EventPreview.


Please review this at https://gwt-code-reviews.appspot.com/1822803/

Affected files:
  M  
reference/code-museum/src/com/google/gwt/museum/client/common/EventReporter.java
  D  
reference/code-museum/src/com/google/gwt/museum/client/defaultmuseum/Issue3186.java
  M  
reference/code-museum/src/com/google/gwt/museum/client/defaultmuseum/VisualsForCheckBoxAndRadioButtonEvents.java
  M  
reference/code-museum/src/com/google/gwt/museum/client/defaultmuseum/VisualsForDialogBox.java
  M  
reference/code-museum/src/com/google/gwt/museum/client/defaultmuseum/VisualsForDisclosurePanelEvents.java
  M  
reference/code-museum/src/com/google/gwt/museum/client/defaultmuseum/VisualsForSuggestBoxEvents.java
  M  
reference/code-museum/src/com/google/gwt/museum/client/defaultmuseum/VisualsForTextEvents.java
  M  
reference/code-museum/src/com/google/gwt/museum/client/defaultmuseum/VisualsForTreeEvents.java

  M reference/dispatch/client/Subject.java
  M tools/api-checker/config/gwt24_25userApi.conf
  M  
tools/benchmark-viewer/src/com/google/gwt/benchmarks/viewer/client/ReportViewer.java

  M user/src/com/google/gwt/event/dom/client/KeyCodes.java
  D user/src/com/google/gwt/user/client/BaseListenerWrapper.java
  M user/src/com/google/gwt/user/client/DOM.java
  M user/src/com/google/gwt/user/client/Event.java
  D user/src/com/google/gwt/user/client/EventPreview.java
  M user/src/com/google/gwt/user/client/History.java
  D user/src/com/google/gwt/user/client/HistoryListener.java
  M user/src/com/google/gwt/user/client/Window.java
  D user/src/com/google/gwt/user/client/WindowCloseListener.java
  D user/src/com/google/gwt/user/client/WindowResizeListener.java
  D user/src/com/google/gwt/user/client/WindowScrollListener.java
  M user/src/com/google/gwt/user/client/impl/HistoryImplSafari.java
  M user/src/com/google/gwt/user/client/ui/Button.java
  D user/src/com/google/gwt/user/client/ui/ChangeListener.java
  D user/src/com/google/gwt/user/client/ui/ChangeListenerCollection.java
  D user/src/com/google/gwt/user/client/ui/ClickListener.java
  D user/src/com/google/gwt/user/client/ui/ClickListenerCollection.java
  M user/src/com/google/gwt/user/client/ui/CustomButton.java
  D  
user/src/com/google/gwt/user/client/ui/DelegatingChangeListenerCollection.java
  D  
user/src/com/google/gwt/user/client/ui/DelegatingClickListenerCollection.java
  D  
user/src/com/google/gwt/user/client/ui/DelegatingFocusListenerCollection.java
  D  
user/src/com/google/gwt/user/client/ui/DelegatingKeyboardListenerCollection.java

  M user/src/com/google/gwt/user/client/ui/DialogBox.java
  D user/src/com/google/gwt/user/client/ui/DisclosureEvent.java
  D user/src/com/google/gwt/user/client/ui/DisclosureHandler.java
  M user/src/com/google/gwt/user/client/ui/DisclosurePanel.java
  D user/src/com/google/gwt/user/client/ui/FiresDisclosureEvents.java
  D user/src/com/google/gwt/user/client/ui/FiresFormEvents.java
  D user/src/com/google/gwt/user/client/ui/FiresSuggestionEvents.java
  D user/src/com/google/gwt/user/client/ui/FocusListener.java
  D user/src/com/google/gwt/user/client/ui/FocusListenerAdapter.java
  D user/src/com/google/gwt/user/client/ui/FocusListenerCollection.java
  M user/src/com/google/gwt/user/client/ui/FocusPanel.java
  M user/src/com/google/gwt/user/client/ui/FocusWidget.java
  D user/src/com/google/gwt/user/client/ui/FormHandler.java
  D user/src/com/google/gwt/user/client/ui/FormHandlerCollection.java
  M user/src/com/google/gwt/user/client/ui/FormPanel.java
  M user/src/com/google/gwt/user/client/ui/HTMLTable.java
  D user/src/com/google/gwt/user/client/ui/HasFocus.java
  D user/src/com/google/gwt/user/client/ui/HasKeyPreview.java
  M user/src/com/google/gwt/user/client/ui/Hyperlink.java
  M user/src/com/google/gwt/user/client/ui/Image.java
  D user/src/com/google/gwt/user/client/ui/KeyboardListener.java
  D user/src/com/google/gwt/user/client/ui/KeyboardListenerAdapter.java
  D user/src/com/google/gwt/user/client/ui/KeyboardListenerCollection.java
  M user/src/com/google/gwt/user/client/ui/Label.java
  M user/src/com/google/gwt/user/client/ui/ListBox.java
  D user/src/com/google/gwt/user/client/ui/ListenerWrapper.java
  D user/src/com/google/gwt/user/client/ui/LoadListener.java
  D user/src/com/google/gwt/user/client/ui/LoadListenerCollection.java
  M user/src/com/google/gwt/user/client/ui/MenuBar.java
  D user/src/com/google/gwt/user/client/ui/MouseListener.java
  D user/src/com/google/gwt/user/client/ui/MouseListenerAdapter.java
  D user/src/com/google/gwt/user/client/ui/MouseListenerCollection.java
  D user/src/com/google/gwt/user/client/ui/MouseWheelListener.java
  D user/src/com/google/gwt/user/client/ui/MouseWheelListenerCollection.java
  D user/src/com/google/gwt/user/client/ui/PopupListener.java
  D user/src/com/google/gwt/user/client/ui/PopupListenerCollection.java
  M 

[gwt-contrib] Remove Windows-specific JNI for the check for updates (issue1820804)

[t . broyer]

Reviewers: cromwellian,

Message:
See
https://groups.google.com/d/topic/google-web-toolkit-contributors/6z9x8S0FYh0/discussion
for the rationale.

Description:
Remove Windows-specific JNI for the check for updates

Remove -Dgwt.devjar in many places, as we no longer have native libs.


Please review this at https://gwt-code-reviews.appspot.com/1820804/

Affected files:
  M build.xml
  M common.ant.xml
  M dev/core/src/com/google/gwt/dev/shell/CheckForUpdates.java
  D dev/core/src/com/google/gwt/dev/shell/ie/CheckForUpdatesIE6.java
  D dev/core/src/com/google/gwt/dev/shell/ie/LowLevelIE6.java
  M distro-source/build.xml
  M eclipse/README.txt
  D eclipse/jni/linux/.checkstyle
  D eclipse/jni/linux/.cproject
  D eclipse/jni/linux/.project
  D eclipse/jni/mac/.cdtproject
  D eclipse/jni/mac/.project
  D eclipse/jni/mac/.settings/org.eclipse.cdt.core.prefs
  M eclipse/plugins/MissingPlugin/MissingPlugin-gwtc.launch
  M eclipse/plugins/MissingPlugin/MissingPlugin.launch
  M eclipse/reference/code-museum/DefaultMuseum.launch
  M eclipse/reference/code-museum/SingleIssue.launch
  M eclipse/samples/DynaTable/DynaTable-gwtc.launch
  M eclipse/samples/Hello/Hello-compModule.launch
  M eclipse/samples/Hello/Hello-gwtc.launch
  M eclipse/samples/Hello/Hello.launch
  M eclipse/samples/JSON/JSON-gwtc.launch
  M eclipse/samples/JSON/JSON.launch
  M eclipse/samples/LogExample/LogExample.gwtc.launch
  M eclipse/samples/LogExample/LogExample.launch
  M eclipse/samples/Mail/Mail-gwtc.launch
  M eclipse/samples/Mail/Mail.launch
  M eclipse/samples/Showcase/Showcase-SSL.launch
  M eclipse/samples/Showcase/Showcase-gwtc.launch
  M eclipse/samples/Showcase/Showcase.launch
  M eclipse/samples/SimpleRPC/SimpleRPC-gwtc.launch
  M eclipse/samples/SimpleRPC/SimpleRPC.launch
  M eclipse/samples/SimpleXML/SimpleXML-gwtc.launch
  M eclipse/samples/SimpleXML/SimpleXML.launch
  D jni/build.xml
  D jni/core/gwt-ll.cpp
  D jni/linux/ExternalWrapper.cpp
  D jni/linux/ExternalWrapper.h
  D jni/linux/JStringWrap.h
  D jni/linux/JsRootedValue.cpp
  D jni/linux/JsRootedValue.h
  D jni/linux/JsStringWrap.h
  D jni/linux/JsValueMoz.cpp
  D jni/linux/LowLevelMoz.cpp
  D jni/linux/Makefile
  D jni/linux/NativeWrapper.cpp
  D jni/linux/Tracer.cpp
  D jni/linux/Tracer.h
  D jni/linux/build.xml
  D jni/linux/gwt-jni.h
  D jni/linux/mozilla-headers.h
  D jni/linux/prebuilt/libgwt-ll.so
  D jni/mac/JStringWrap.h
  D jni/mac/Makefile
  D jni/mac/build.xml
  D jni/mac/gwt-webkit.cpp
  D jni/mac/java-dispatch.cpp
  D jni/mac/java-dispatch.h
  D jni/mac/org.eclipse.swt/webkit.c
  D jni/mac/prebuilt/libgwt-ll.jnilib
  D jni/mac/trace.cpp
  D jni/mac/trace.h
  D jni/windows/IE6.cpp
  D jni/windows/Makefile
  D jni/windows/build.xml
  D jni/windows/gwt-ll.vcproj
  D jni/windows/prebuilt/gwt-ll.dll
  D jni/windows/wininet.def
  D jni/windows/wininet.h


--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Remove long-deprecated GWTShell and GWTCompiler tools. (issue1821804)

[t . broyer]

Reviewers: cromwellian,

Description:
Remove long-deprecated GWTShell and GWTCompiler tools.

Added a test to make sure JSP works.


Please review this at https://gwt-code-reviews.appspot.com/1821804/

Affected files:
  M dev/build.xml
  M dev/core/src/com/google/gwt/dev/CompileTaskRunner.java
  D dev/core/src/com/google/gwt/dev/GWTCompiler.java
  D dev/core/src/com/google/gwt/dev/GWTShell.java
  M dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java
  D dev/core/src/com/google/gwt/dev/etc/tomcat/conf/web.xml
  D dev/core/src/com/google/gwt/dev/etc/tomcat/webapps/ROOT/WEB-INF/web.xml
  D dev/core/src/com/google/gwt/dev/shell/GWTShellServlet.java
  D dev/core/src/com/google/gwt/dev/shell/HostedModeServletContextProxy.java
  D dev/core/src/com/google/gwt/dev/shell/tomcat/CatalinaLoggerAdapter.java
  D dev/core/src/com/google/gwt/dev/shell/tomcat/CommonsLoggerAdapter.java
  D dev/core/src/com/google/gwt/dev/shell/tomcat/EmbeddedTomcatServer.java
  D dev/core/src/org/apache/COPYING
  D dev/core/src/org/apache/catalina/loader/WebappClassLoader.java
  D dev/core/test/com/google/gwt/dev/GWTCompilerTest.java
  D dev/core/test/com/google/gwt/dev/GWTShellTest.java
  M eclipse/dev/.classpath
  A user/test/com/google/gwt/dev/shell/jsp/JspTest.gwt.xml
  A user/test/com/google/gwt/dev/shell/jsp/client/JspTest.java
  A user/test/com/google/gwt/dev/shell/jsp/public/test.jsp


--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Remove long-deprecated GWTShell and GWTCompiler tools. (issue1821804)

[t . broyer]

https://gwt-code-reviews.appspot.com/1821804/diff/1/user/test/com/google/gwt/dev/shell/jsp/client/JspTest.java
File user/test/com/google/gwt/dev/shell/jsp/client/JspTest.java (right):

https://gwt-code-reviews.appspot.com/1821804/diff/1/user/test/com/google/gwt/dev/shell/jsp/client/JspTest.java#newcode28
user/test/com/google/gwt/dev/shell/jsp/client/JspTest.java:28: * @see
http://code.google.com/p/google-web-toolkit/issues/detail?id=3557
Note: I didn't fix the issue, as I think (judging by the comments on the
issue) that the proper fix would be to update to Jasper 2, and there
should probably be a command-line flag to set the source compatibility
(given that this is done from the ServletContainerLauncher, it would
have to be an argument to the JettyLauncher, e.g. -server :source=1.5).
Updating the dependency will be easier once we switch to Maven.

https://gwt-code-reviews.appspot.com/1821804/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Patch for issue #2467: Tree scrolls horizontally when an item is selected (issue1819803)

[t . broyer]

http://gwt-code-reviews.appspot.com/1819803/diff/1/user/src/com/google/gwt/user/client/ui/Tree.java
File user/src/com/google/gwt/user/client/ui/Tree.java (right):

http://gwt-code-reviews.appspot.com/1819803/diff/1/user/src/com/google/gwt/user/client/ui/Tree.java#newcode641
user/src/com/google/gwt/user/client/ui/Tree.java:641: * Determines
whether scrolling a tree item into view is currently enabled.
whether selecting a tree item will scroll it into view (or something
similar)

http://gwt-code-reviews.appspot.com/1819803/diff/1/user/src/com/google/gwt/user/client/ui/Tree.java#newcode643
user/src/com/google/gwt/user/client/ui/Tree.java:643: * @return true if
scrolling a tree item into view is enabled, false if not
We generally avoid stating the obvious in JavaDoc.

http://gwt-code-reviews.appspot.com/1819803/diff/1/user/src/com/google/gwt/user/client/ui/Tree.java#newcode872
user/src/com/google/gwt/user/client/ui/Tree.java:872: public void
setScrollIntoViewEnabled(boolean enable) {
setScrollTreeItemIntoViewOnSelect would be a better name, or maybe
setTreeItemScrolledIntoViewOnSelect for the getter to look better:
isTreeItemScrolledIntoViewOnSelect

http://gwt-code-reviews.appspot.com/1819803/diff/1/user/src/com/google/gwt/user/client/ui/Tree.java#newcode1275
user/src/com/google/gwt/user/client/ui/Tree.java:1275: if
(scrollIntoViewEnabled) {
All of the above is about moving the 'focusable' element so that
scrollIntoView works. If we don't scrollIntoView(focusable), then we
don't need to compute the location of the selected item and move the
focusable element around. Or is there a risk that setFocus (which sets
the focus on the 'focusable' element) would scroll it into view too, in
some browsers and this computation is needed anyway?

(as a side note, I wonder whether it's an oversight or on-purpose that
updateAriaAttributes isn't called in the 'focusableWidget != null' case;
similarly, the 'tree' role should probably be set on getElement() rather
than 'focusable'; but A11Y is another issue)

http://gwt-code-reviews.appspot.com/1819803/diff/1/user/test/com/google/gwt/user/client/ui/TreeTest.java
File user/test/com/google/gwt/user/client/ui/TreeTest.java (right):

http://gwt-code-reviews.appspot.com/1819803/diff/1/user/test/com/google/gwt/user/client/ui/TreeTest.java#newcode348
user/test/com/google/gwt/user/client/ui/TreeTest.java:348: public void
testScrollIntoViewEnabled() {
This test is totally useless.

Testing whether the flag has a side-effect would be useful:
 1. create a ScrollPanel with a very small width and height,
 2. put the Tree inside and populate it with a bunch of TreeItems,
 3. make sure the ScrollPanel is at 0,0
 4. select the innermost TreeItem
 5. verify that the ScrollPanel has scrolled
 6. setScrollTreeItemIntoViewOnSelect(false) and repeat steps 3-5, but
check that the ScrollPanel has *not* scrolled in this case.

http://gwt-code-reviews.appspot.com/1819803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Use c.g.g.core.shared.GWT in shared code. (issue1818803)

[t . broyer]

Reviewers: skybrian,

Message:
For inclusion in GWT 2.5

Description:
Use c.g.g.core.shared.GWT in shared code.

Issue 7527

Please review this at https://gwt-code-reviews.appspot.com/1818803/

Affected files:
  M user/src/com/google/gwt/dom/builder/shared/ElementBuilderFactory.java
  M user/src/com/google/gwt/dom/builder/shared/ElementBuilderImpl.java
  M user/src/com/google/gwt/dom/builder/shared/HtmlStylesBuilder.java
  M user/src/com/google/gwt/place/shared/PlaceController.java
  M user/src/com/google/gwt/place/shared/PlaceHistoryHandler.java
  M user/src/com/google/gwt/place/shared/PlaceHistoryMapper.java
  M user/src/com/google/gwt/safecss/shared/SafeStylesHostedModeUtils.java
  M user/src/com/google/gwt/safecss/shared/SafeStylesUtils.java
  M user/src/com/google/gwt/safehtml/shared/SafeHtmlHostedModeUtils.java
  M user/src/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java
  M user/src/com/google/gwt/safehtml/shared/UriUtils.java
  M user/src/com/google/web/bindery/autobean/shared/ValueCodexHelper.java


Index: user/src/com/google/gwt/dom/builder/shared/ElementBuilderFactory.java
diff --git  
a/user/src/com/google/gwt/dom/builder/shared/ElementBuilderFactory.java  
b/user/src/com/google/gwt/dom/builder/shared/ElementBuilderFactory.java
index  
31da8b1480277ebac5fde0edaa99c23093fd89eb..76e34a051f0381ae76d912b9ec421782dfba0c38  
100644

--- a/user/src/com/google/gwt/dom/builder/shared/ElementBuilderFactory.java
+++ b/user/src/com/google/gwt/dom/builder/shared/ElementBuilderFactory.java
@@ -15,7 +15,7 @@
  */
 package com.google.gwt.dom.builder.shared;

-import com.google.gwt.core.client.GWT;
+import com.google.gwt.core.shared.GWT;

 /**
  * Factory for creating element builders.
Index: user/src/com/google/gwt/dom/builder/shared/ElementBuilderImpl.java
diff --git  
a/user/src/com/google/gwt/dom/builder/shared/ElementBuilderImpl.java  
b/user/src/com/google/gwt/dom/builder/shared/ElementBuilderImpl.java
index  
131e4de96b696aa601deaedbbcf4b6f3765f2e4e..1f6949328e0dae30b860baa167a1f9826c2b393b  
100644

--- a/user/src/com/google/gwt/dom/builder/shared/ElementBuilderImpl.java
+++ b/user/src/com/google/gwt/dom/builder/shared/ElementBuilderImpl.java
@@ -15,7 +15,7 @@
  */
 package com.google.gwt.dom.builder.shared;

-import com.google.gwt.core.client.GWT;
+import com.google.gwt.core.shared.GWT;
 import com.google.gwt.dom.client.Element;
 import com.google.gwt.regexp.shared.RegExp;
 import com.google.gwt.safehtml.shared.SafeHtml;
Index: user/src/com/google/gwt/dom/builder/shared/HtmlStylesBuilder.java
diff --git  
a/user/src/com/google/gwt/dom/builder/shared/HtmlStylesBuilder.java  
b/user/src/com/google/gwt/dom/builder/shared/HtmlStylesBuilder.java
index  
2c1276951ae3788d038d1e2719c852aa299d4669..ce8cd98021e5e839d8b8e37a8de267f198e18de8  
100644

--- a/user/src/com/google/gwt/dom/builder/shared/HtmlStylesBuilder.java
+++ b/user/src/com/google/gwt/dom/builder/shared/HtmlStylesBuilder.java
@@ -15,8 +15,8 @@
  */
 package com.google.gwt.dom.builder.shared;

-import com.google.gwt.core.client.GWT;
 import com.google.gwt.core.client.JavaScriptObject;
+import com.google.gwt.core.shared.GWT;
 import com.google.gwt.dom.client.Style.BorderStyle;
 import com.google.gwt.dom.client.Style.Cursor;
 import com.google.gwt.dom.client.Style.Display;
Index: user/src/com/google/gwt/place/shared/PlaceController.java
diff --git a/user/src/com/google/gwt/place/shared/PlaceController.java  
b/user/src/com/google/gwt/place/shared/PlaceController.java
index  
21b80eb2b41f18beaa8d2a1d3839889f7668dee4..a34d8178f84e037a1df5b8dbb2f86d1046f215fd  
100644

--- a/user/src/com/google/gwt/place/shared/PlaceController.java
+++ b/user/src/com/google/gwt/place/shared/PlaceController.java
@@ -15,7 +15,7 @@
  */
 package com.google.gwt.place.shared;

-import com.google.gwt.core.client.GWT;
+import com.google.gwt.core.shared.GWT;
 import com.google.gwt.event.shared.HandlerRegistration;
 import com.google.gwt.user.client.Window;
 import com.google.gwt.user.client.Window.ClosingEvent;
Index: user/src/com/google/gwt/place/shared/PlaceHistoryHandler.java
diff --git a/user/src/com/google/gwt/place/shared/PlaceHistoryHandler.java  
b/user/src/com/google/gwt/place/shared/PlaceHistoryHandler.java
index  
af138bde5f0c2f2947e75d155a80295c10770bb4..e0b37bd92093c0e9c7732d73a75bdf605ed3048b  
100644

--- a/user/src/com/google/gwt/place/shared/PlaceHistoryHandler.java
+++ b/user/src/com/google/gwt/place/shared/PlaceHistoryHandler.java
@@ -15,7 +15,7 @@
  */
 package com.google.gwt.place.shared;

-import com.google.gwt.core.client.GWT;
+import com.google.gwt.core.shared.GWT;
 import com.google.gwt.event.logical.shared.ValueChangeEvent;
 import com.google.gwt.event.logical.shared.ValueChangeHandler;
 import com.google.gwt.event.shared.LegacyHandlerWrapper;
Index: user/src/com/google/gwt/place/shared/PlaceHistoryMapper.java
diff --git a/user/src/com/google/gwt/place/shared/PlaceHistoryMapper.java  

[gwt-contrib] Re: Use c.g.g.core.shared.GWT in shared code. (issue1818803)

[t . broyer]

On 2012/08/28 13:17:58, jtamplin wrote:

LGTM, as far as it goes



However, when you use GWT.create in non-client code, there is some

setup you

need to do - you have to create the ServerGwtBridge, set the deferred

binding

properties (either globally or per-thread), and you need to register

any

ClassInstantiators you need for the various GWT.create calls.  Look at
ServerGwtBridgeTest for some examples.



I haven't looked at the usages in these files, but basically the only

things

GWT.create knows how to do out of the box right now on the server is

to find the

local-specific classes for subtypes of Localizable and to do new

FooImpl() or

new Foo() for GWT.create(Foo.class).



Most likely, you will also need to create and register additional

class

instantiators for the GWT.create usages here, and then document what

additional

properties need to be setup for server-side usage.


This is only about fixing NoClassDefFoundErrors on server-side, as
c.g.g.client.GWT is not included in gwt-servlet.jar.

In most cases, it's only used for GWT.isClient() or GWT.isScript() to
switch strategy depending on whether it's in a VM and/or DevMode, or
compiled to JS. Most uses of GWT.create are conditionned by such tests,
so won't be an issue. There are a couple uses of GWT.create without
those checks though, but they're only there for convenience and wouldn't
work outside DevMode anyway:
 - PlaceController's Delegate to default to DefaultDelegate, which
relies on c.g.g.u.c.Window.prompt()
 - PlaceHistoryHandler's Historian to default to DefaultHistorian, which
relies on c.g.g.u.c.History


https://gwt-code-reviews.appspot.com/1818803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Ensures integer pixel values and adds getters for subpixel values (issue1815803)

[t . broyer]

Are there tests that could be added / updated?


http://gwt-code-reviews.appspot.com/1815803/diff/1/src/com/google/gwt/dom/client/DOMImpl.java
File src/com/google/gwt/dom/client/DOMImpl.java (right):

http://gwt-code-reviews.appspot.com/1815803/diff/1/src/com/google/gwt/dom/client/DOMImpl.java#newcode149
src/com/google/gwt/dom/client/DOMImpl.java:149: public int
eventGetScreenX(NativeEvent evt) {
Should these be made 'final' to make sure subclasses only ever override
the subpixel variants?

http://gwt-code-reviews.appspot.com/1815803/diff/1/src/com/google/gwt/dom/client/DOMImplStandard.java
File src/com/google/gwt/dom/client/DOMImplStandard.java (right):

http://gwt-code-reviews.appspot.com/1815803/diff/1/src/com/google/gwt/dom/client/DOMImplStandard.java#newcode61
src/com/google/gwt/dom/client/DOMImplStandard.java:61: screenY|0,
clientX|0, clientY|0, ctrlKey, altKey, shiftKey, metaKey, button,
How about changing the argument types to `double` instead? (or adding an
overload if we're concerned about binary-compatibility)

`NativeEvent`s generated by the browser can very well contain subpixel
coordinates, so we should be able to generate them from GWT too.
And getting the coordinates out the event would return integer values
anyway (unless you use the subpixel variant), so it's a safe move.

http://gwt-code-reviews.appspot.com/1815803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Force returning integer pixel values from DOMImplWebkit (#6130) (issue1797805)

[t . broyer]

absoluteTop/Left are not enough:
http://code.google.com/p/google-web-toolkit/issues/detail?id=7575

And WebKit is probably not enough too (subpixel rendering is becoming
standard, and will soon be mainstream).

http://gwt-code-reviews.appspot.com/1797805/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Move GAE Auth functionality from Expenses over the MobileWebApp sample. (issue1806803)

[t . broyer]

FYI, my take on authentication and RequestFactory can be found at
https://github.com/tbroyer/gwt-maven-archetypes/tree/master/guice-rf-activities
(also uses Guice, GIN and Guava).
I'm in the process of trading the userName and isAdmin JS variables for
an object (managing ser/deserialization with AutoBeans) and logout (with
a configurable logout URL, through JNDI).
I'm using it to have pluggable authentication (static file, LDAP or
database) and authentication mechanism (defaults to form auth, but can
be changed to SSL client certificate or NTLM/SP-NEGO –in which case
you'd configure the logout URL to the empty string to disable logout–)
at the servlet container level.
The code should be easy to migrate to GAE.

http://gwt-code-reviews.appspot.com/1806803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

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

[t . broyer]

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 CL fixes it wrt JDK 7, as
all I see is JavaScript / client code; if the issue is that HTMLUnit
doesn't follow the specs but rely on Java's Unicode support, then it
should be documented that this is a workaround for an HTMLUnit bug)

http://gwt-code-reviews.appspot.com/1796803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fixes issue 6653: Activity interface should use the new EventBus (issue1786804)

[t . broyer]

On 2012/07/25 10:01:17, manolo.carrasco wrote:

 A bit of discipline maybe, but no pain at all (not for me at

least).


You are right s/pain/discipline/.



When you use gin for places and activities you have to be aware of

this

circumstance to avoid  multiple instances of the event bus, so you

have to write

a bit more code


Huh!
How about simply letting the GIN generation fail if some class depends
on the gwt.event EventBus rather than the web.bindery one? (EventBus
being abstract, GIN won't be able to instantiate it and will defer to
GWT.create(), which will fail too).

Put simply: do not ever, never use com.google.gwt.event.shared.EventBus
in your code and it'll Just Work™; and if you do, it should fail.

BTW, you code can be simplified to:

   bind(com.google.web.bindery.event.shared.EventBus.class)
  .to(com.google.gwt.event.shared.SimpleEventBus.class);
   bind(com.google.gwt.event.shared.EventBus.class)
  .to(com.google.gwt.event.shared.SimpleEventBus.class);
   bind(com.google.gwt.event.shared.SimpleEventBus.class)
  .in(Singleton.class);

(you want the SimpleEventBus as a singleton, not each one of the
EventBus-es, which is what your code that doesn't work was declaring)

But it's not really the place to discuss this ;-)

http://gwt-code-reviews.appspot.com/1786804/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors