Summary: | REGRESSION: Multipart image documents are garbled | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | mitz | ||||||||
Component: | Page Loading | Assignee: | 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
mitz
2007-05-17 15:58:44 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.
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 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 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 on attachment 14642 [details]
Patch including http test
ImageDocument change, so this patch no longer applies cleanly. I'm going to update it.
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 on attachment 14716 [details]
Patch including http test
r=me
|