Bug 13767

Summary: REGRESSION: Multipart image documents are garbled
Product: WebKit Reporter: mitz
Component: Page LoadingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal Keywords: InRadar, Regression
Priority: P1    
Version: 523.x (Safari 3)   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
Patch
none
Patch including http test
none
Patch including http test darin: review+

Description mitz 2007-05-17 15:58:44 PDT
This is the standalone image document of bug 13759. The symptoms and the root cause are the same: the image uses the resource data, which is then overwritten by the next part.
Comment 1 mitz 2007-05-17 16:00:57 PDT
Created attachment 14599 [details]
Patch

I tried to make a test, for which I included the standalone image document in an iframe. The problem is that it asserts in FrameLoader::addData(). I'll need to investigate that before I can complete this patch.
Comment 2 Brady Eidson 2007-05-17 16:02:30 PDT
<rdar://problem/5211789>
Comment 3 Brady Eidson 2007-05-17 16:09:10 PDT
Patch is a great start (I know the layout test is going to be more work)

One comment - generally using a PassRefPtr as a local variable is a bad idea and can lead to unpredictable problems later.  While this specific case would be fine, no harm in using a RefPtr instead
Comment 4 mitz 2007-05-21 08:42:43 PDT
Created attachment 14642 [details]
Patch including http test

This patch includes a version of the test that does not trigger the assertion failure (which I am going to file a separate bug on).
Comment 5 Darin Adler 2007-05-22 19:37:12 PDT
Comment on attachment 14642 [details]
Patch including http test

+        cachedImage->data(data, true);

Should be data.release() instead of just data to avoid reference count churn.

Need to remove extra ChangeLog entry in the LayoutTests directory.

r=me, because neither of those is a real problem
Comment 6 mitz 2007-05-25 02:28:13 PDT
Comment on attachment 14642 [details]
Patch including http test

ImageDocument change, so this patch no longer applies cleanly. I'm going to update it.
Comment 7 mitz 2007-05-25 03:02:32 PDT
Created attachment 14716 [details]
Patch including http test

Updated for TOT and addressed Darin's comments. Expected test results have also changed because standalone image documents are in quirks mode now.
Comment 8 Darin Adler 2007-05-25 08:05:52 PDT
Comment on attachment 14716 [details]
Patch including http test

r=me
Comment 9 Sam Weinig 2007-05-25 10:38:40 PDT
Landed in r21760.