Summary: | REGRESSION (r30587): Document::implicitClose() not called when done loading page (Image does not get scaled to fit) | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Oliver Hunt <oliver> | ||||
Component: | Page Loading | Assignee: | mitz | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | bdakin, hyatt, mitz, oliver | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Mac | ||||||
OS: | OS X 10.5 | ||||||
URL: | http://factoryjoe.com/blog/2007/08/25/groups-for-twitter-or-a-proposal-for-twitter-tag-channels/ | ||||||
Attachments: |
|
Description
Oliver Hunt
2008-03-04 04:13:56 PST
Looking in inspector one difference i noticed was that there was no listing for the img elements inline style... The script library the page uses does not alter the inline style when running in Safari. I hit this assertion failure when navigating away from this page: void FrameView::scheduleRelayout() { ASSERT(!m_frame->document() || !m_frame->document()->inPageCache()); I cannot reproduce either problem with a low-bandwidth connection. Setting a breakpoint at Document::implicitClose() confirms that the load event is never dispatched when the page finishes loading, thus the script that's supposed to resize the images doesn't run. The load event *is* dispatched when you navigate out. By then the page is in the page cache and scripts really should not be executing. The event handler runs, resizes the image, and triggers a layout of a now-cached page, which causes the assertion failure. Indeed, you can force the images to resize by entering javascript: document.close() in the address bar after the page has finished loading. The page links to a 404 style sheet, <http://factoryjoe.com/blog/wp-content/plugins/diso-profile/profile.css>, so I highly suspect <http://trac.webkit.org/projects/webkit/changeset/30587>. Created attachment 19526 [details]
Call setLoadInProgress(false) in the 4xx case
Comment on attachment 19526 [details]
Call setLoadInProgress(false) in the 4xx case
r=me, though what's with "\ No newline at end of file" in the middle of the testcase?
r=me too, but I noticed another potential problem: do we need to surround the call to object->error() inside Loader::didReceiveData with calls to setLoadInProgress(true) and then setLoadInProgress(false)? (In reply to comment #10) > r=me too, but I noticed another potential problem: do we need to surround the > call to object->error() inside Loader::didReceiveData with calls to > setLoadInProgress(true) and then setLoadInProgress(false)? I think so but I do not know how to make a test case to prove it. (In reply to comment #9) > (From update of attachment 19526 [details] [edit]) > r=me, though what's with "\ No newline at end of file" in the middle of the > testcase? There used not to be a newline, now there should be. |