[webkit-reviews] review denied: [Bug 22562] REGRESSION (r37971): events not firing after going back in back/forward cache : [Attachment 25981] Proposed patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Dec 12 09:16:34 PST 2008

Darin Adler <darin at apple.com> has denied Cameron Zwarich (cpst)
<cwzwarich at uwaterloo.ca>'s request for review:
Bug 22562: REGRESSION (r37971): events not firing after going back in
back/forward cache

Attachment 25981: Proposed patch

------- Additional Comments from Darin Adler <darin at apple.com>
This is not quite the right place for this. If you look in FrameLoader::open
you'll see that before calling CachedPage::restore, there are a couple calls on
m_frame that change fields in the domWindow. So it's too late to change the
DOMWindow after that code has already run. We probably should set the DOMWindow
at almost exactly the same time we set the document. I don't really have an
opinion about what code should go inside CachedPage::restore vs. what code goes
in FrameLoader::open -- CachedPage::restore is only called from that one
function, and the two work together without a clear division of
responsibilities -- but clearly there's an order dependency that we have to get

Also, if you add a function the name should be setDOMWindow, not setDomWindow.

I think it's important in the longer term to have some regression tests for
this, as you have mentioned before.

More information about the webkit-reviews mailing list