RESOLVED FIXED 17667
REGRESSION (r30587): Document::implicitClose() not called when done loading page (Image does not get scaled to fit)
https://bugs.webkit.org/show_bug.cgi?id=17667
Summary REGRESSION (r30587): Document::implicitClose() not called when done loading p...
Oliver Hunt
Reported 2008-03-04 04:13:56 PST
The image at the top of the post is not correctly scaled
Attachments
Call setLoadInProgress(false) in the 4xx case (3.58 KB, patch)
2008-03-04 10:38 PST, mitz
oliver: review+
Oliver Hunt
Comment 1 2008-03-04 04:22:12 PST
Looking in inspector one difference i noticed was that there was no listing for the img elements inline style...
mitz
Comment 2 2008-03-04 07:14:44 PST
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());
mitz
Comment 3 2008-03-04 08:48:57 PST
I cannot reproduce either problem with a low-bandwidth connection.
mitz
Comment 4 2008-03-04 09:27:30 PST
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.
mitz
Comment 5 2008-03-04 09:30:45 PST
Indeed, you can force the images to resize by entering javascript: document.close() in the address bar after the page has finished loading.
mitz
Comment 6 2008-03-04 10:13:45 PST
mitz
Comment 7 2008-03-04 10:29:58 PST
mitz
Comment 8 2008-03-04 10:38:15 PST
Created attachment 19526 [details] Call setLoadInProgress(false) in the 4xx case
Oliver Hunt
Comment 9 2008-03-04 12:03:06 PST
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?
Geoffrey Garen
Comment 10 2008-03-04 12:04:25 PST
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)?
mitz
Comment 11 2008-03-04 12:16:57 PST
(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.
mitz
Comment 12 2008-03-04 12:18:05 PST
(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.
mitz
Comment 13 2008-03-04 13:13:57 PST
Note You need to log in before you can comment on or make changes to this bug.