a code review - issue2014_r2972.patch

7 views
Skip to first unread message

Joel Webber

unread,
May 30, 2008, 6:22:21 PM5/30/08
to Google Web Toolkit Contributors, Scott Blum
Scott,

Can you have a look at this change for me? It fixes a typo in the Window.init() method that was causing onunload, etc. to not get cleaned up on shutdown. This does not seem to have been causing problems anywhere except when *closing* a window on IE (not refreshing or navigating away). Go figure.

A couple of notes:
- I took a moment to revisit the reason for __gwt_initHandlers() in the first place. It turns out that when the script is running in an iframe, it can't set window.onbeforeunload, at least on IE). So its existence is still justified.
- I moved the cleanup code into the selection-script templates, because it didn't make sense for it to be where it was (and onbeforeunload can't be set from within the script frame anyway).

Affected files:
M      dev/core/src/com/google/gwt/core/ext/linker/impl/HostedModeTemplate.js
M      dev/core/src/com/google/gwt/core/linker/XSTemplate.js
M      dev/core/src/com/google/gwt/core/linker/IFrameTemplate.js
M      dev/core/src/com/google/gwt/core/linker/SingleScriptTemplate.js
M      user/src/com/google/gwt/user/client/Window.java

issue2014_r2972.patch

Scott Blum

unread,
May 30, 2008, 6:28:25 PM5/30/08
to Joel Webber, Google Web Toolkit Contributors
Sure, is this for RC2 or trunk, btw?

Scott Blum

unread,
May 30, 2008, 6:30:23 PM5/30/08
to Joel Webber, Google Web Toolkit Contributors
LGTMBTW

Joel Webber

unread,
Jun 2, 2008, 10:15:16 AM6/2/08
to Scott Blum, Google Web Toolkit Contributors
yes.

On Fri, May 30, 2008 at 6:28 PM, Scott Blum <sco...@google.com> wrote:

Scott Blum

unread,
Jun 2, 2008, 6:48:15 PM6/2/08
to Joel Webber, Google Web Toolkit Contributors
Aight, just put it into the RC2 branch then, and we'll merge things into trunk en masse + on demand.

Joel Webber

unread,
Jun 3, 2008, 10:57:17 AM6/3/08
to Scott Blum, Google Web Toolkit Contributors
Committed (to 1.5 branch) as r2981.
Reply all
Reply to author
Forward
0 new messages