[gwt-contrib] Re: Fix bug on IE where onload events don't fire for IFrames. (issue1294801)

2011-03-14 Thread jlabanca 


http://gwt-code-reviews.appspot.com/1294801/diff/17001/user/src/com/google/gwt/user/client/ui/Frame.java
File user/src/com/google/gwt/user/client/ui/Frame.java (right):

http://gwt-code-reviews.appspot.com/1294801/diff/17001/user/src/com/google/gwt/user/client/ui/Frame.java#newcode78
user/src/com/google/gwt/user/client/ui/Frame.java:78:
sinkEvents(Event.ONLOAD);
Don't sink in the constructor unless its used by the Widget itself.
addDomHandler will lazily sink the load event automatically.  This is
important because sinking events is expensive.

http://gwt-code-reviews.appspot.com/1294801/diff/17001/user/src/com/google/gwt/user/client/ui/Frame.java#newcode110
user/src/com/google/gwt/user/client/ui/Frame.java:110: return
addHandler(handler, LoadEvent.getType());
Use addDomHandler() for events that wrap native events.  It will sink
ONLOAD the first time it is called.

http://gwt-code-reviews.appspot.com/1294801/diff/17001/user/test/com/google/gwt/dom/public-test/iframetest.html
File user/test/com/google/gwt/dom/public-test/iframetest.html (right):

http://gwt-code-reviews.appspot.com/1294801/diff/17001/user/test/com/google/gwt/dom/public-test/iframetest.html#newcode3
user/test/com/google/gwt/dom/public-test/iframetest.html:3: /html
This doesn't look right

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

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

[gwt-contrib] Re: Fix bug on IE where onload events don't fire for IFrames. (issue1294801)

2011-03-14 Thread jlabanca 

LGTM

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

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

[gwt-contrib] Re: Fix bug on IE where onload events don't fire for IFrames. (issue1294801)

2011-03-14 Thread pdr 

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

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

[gwt-contrib] Re: Fix bug on IE where onload events don't fire for IFrames. (issue1294801)

2011-03-14 Thread jlabanca 

iframetest.html still looks weird.  I'll assume it looks correct on your
system.

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

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

[gwt-contrib] Re: Fix bug on IE where onload events don't fire for IFrames. (issue1294801)

2011-03-14 Thread pdr 


http://gwt-code-reviews.appspot.com/1294801/diff/17001/user/src/com/google/gwt/user/client/ui/Frame.java
File user/src/com/google/gwt/user/client/ui/Frame.java (right):

http://gwt-code-reviews.appspot.com/1294801/diff/17001/user/src/com/google/gwt/user/client/ui/Frame.java#newcode78
user/src/com/google/gwt/user/client/ui/Frame.java:78:
sinkEvents(Event.ONLOAD);
On 2011/03/14 15:10:05, jlabanca wrote:

Don't sink in the constructor unless its used by the Widget itself.
addDomHandler will lazily sink the load event automatically.  This is

important

because sinking events is expensive.


Done.

http://gwt-code-reviews.appspot.com/1294801/diff/17001/user/src/com/google/gwt/user/client/ui/Frame.java#newcode110
user/src/com/google/gwt/user/client/ui/Frame.java:110: return
addHandler(handler, LoadEvent.getType());
On 2011/03/14 15:10:05, jlabanca wrote:

Use addDomHandler() for events that wrap native events.  It will sink

ONLOAD the

first time it is called.


Done.

http://gwt-code-reviews.appspot.com/1294801/diff/17001/user/test/com/google/gwt/dom/public-test/iframetest.html
File user/test/com/google/gwt/dom/public-test/iframetest.html (right):

http://gwt-code-reviews.appspot.com/1294801/diff/17001/user/test/com/google/gwt/dom/public-test/iframetest.html#newcode3
user/test/com/google/gwt/dom/public-test/iframetest.html:3: /html
On 2011/03/14 15:10:05, jlabanca wrote:

This doesn't look right

svn diff!! :(

fixed :)

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

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

[gwt-contrib] Re: Fix bug on IE where onload events don't fire for IFrames. (issue1294801)

2011-03-14 Thread pdr 

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

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

[gwt-contrib] Re: Fix bug on IE where onload events don't fire for IFrames. (issue1294801)

2011-03-14 Thread jlabanca 

LGTM

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

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

[gwt-contrib] Re: Fix bug on IE where onload events don't fire for IFrames. (issue1294801)

2011-03-13 Thread pdr 

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

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

[gwt-contrib] Re: Fix bug on IE where onload events don't fire for IFrames. (issue1294801)

2011-01-18 Thread pdr 

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

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

[gwt-contrib] Re: Fix bug on IE where onload events don't fire for IFrames. (issue1294801)

2011-01-18 Thread pdr 


http://gwt-code-reviews.appspot.com/1294801/diff/3001/4002
File user/test/com/google/gwt/dom/client/FrameTests.java (right):

http://gwt-code-reviews.appspot.com/1294801/diff/3001/4002#newcode49
user/test/com/google/gwt/dom/client/FrameTests.java:49: final Timer
timer = new Timer() {
On 2011/01/14 19:14:48, jlabanca wrote:

Do you need this timer?  The test will time out if onload is never

triggered.

Done.

http://gwt-code-reviews.appspot.com/1294801/diff/3001/4002#newcode75
user/test/com/google/gwt/dom/client/FrameTests.java:75:
delayTestFinish(2 * delayMillis);
On 2011/01/14 19:14:48, jlabanca wrote:

Move this above the line where you create the iframe to avoid timing

issues,

especially in HtmlUnit.  If the iframe happens to load synchronously

or very

quickly, the onload event might fire before delayTestFinish.  In

general, always

call delayTestFinish() before calling the thing that will trigger an
asynchronous event.


Done.

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

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

[gwt-contrib] Re: Fix bug on IE where onload events don't fire for IFrames. (issue1294801)

2011-01-18 Thread jlabanca 

LGTM

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

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

[gwt-contrib] Re: Fix bug on IE where onload events don't fire for IFrames. (issue1294801)

2011-01-14 Thread pdr 

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

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

[gwt-contrib] Re: Fix bug on IE where onload events don't fire for IFrames. (issue1294801)

2011-01-14 Thread jlabanca 


http://gwt-code-reviews.appspot.com/1294801/diff/3001/4002
File user/test/com/google/gwt/dom/client/FrameTests.java (right):

http://gwt-code-reviews.appspot.com/1294801/diff/3001/4002#newcode49
user/test/com/google/gwt/dom/client/FrameTests.java:49: final Timer
timer = new Timer() {
Do you need this timer?  The test will time out if onload is never
triggered.

http://gwt-code-reviews.appspot.com/1294801/diff/3001/4002#newcode75
user/test/com/google/gwt/dom/client/FrameTests.java:75:
delayTestFinish(2 * delayMillis);
Move this above the line where you create the iframe to avoid timing
issues, especially in HtmlUnit.  If the iframe happens to load
synchronously or very quickly, the onload event might fire before
delayTestFinish.  In general, always call delayTestFinish() before
calling the thing that will trigger an asynchronous event.

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

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