Closed
Bug 9336
Opened 25 years ago
Closed 25 years ago
NetContext[A]Sync ignores error codes from imagelib
Categories
(Core :: Layout, defect, P3)
Tracking
()
VERIFIED
FIXED
M10
People
(Reporter: newt, Assigned: pnunn)
References
()
Details
(Whiteboard: could be in gfx?)
The image referenced in the URL has an invalid PNG line filter about 75% of the
way in, which causes libpng to abort. Even when il_png_write() is modified to
return an error value (-1) to its caller, the error code simply gets passed on
up the chain until it reaches either ImageConsumer::OnDataAvailable()
(reader->Write()) or ImageNetContextSyncImpl::GetURL() (aReader->Write()) and is
ignored. As a result, il_png_write() gets called an additional 27 times in this
case.
I'm not 100% certain there's not another abort mechanism I should be using in
lieu of the error return code (maybe IL_ABORTED or IC_ABORT_PENDING?), but it
seems fairly obvious from the FirstWrite() code immediately above that Write()'s
return code could and should be handled in a similar manner--that is, something
like this:
ilErr = reader->Write(...);
if (ilErr != 0) {
result = NS_ERROR_ABORT;
break;
}
This will help speed up error-recovery, obviously.
Reporter | ||
Comment 1•25 years ago
|
||
I should note that the test image will crash the browser if the fix to 924 is
not in place. I supplied both a mini-patch that takes care of that and a major
rewrite of the pngcom code that does so more cleanly, but neither has been
checked in as of this evening.
In addition, I note that the gfx/src code treats 0 as the "OK" value (which is
standard), while the legacy PNG code uses 1. I assume the latter is what needs
to be fixed.
Finally, my example above is sort of a hybrid of the proper code for each of the
two locations; the correct code in each case should be obvious from context
(i.e., copy the FirstWrite() code).
Kevin,
I'm happy to consider this code my side of the fence.
I'm reassigning to me. If you'd like to keep it, let me
know.
-pnunn
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Updated•25 years ago
|
Status: RESOLVED → REOPENED
Comment 4•25 years ago
|
||
With the August 11th build (1999081108), the program crashs on Mac 8.6, Win NT,
and Win 98 when attempting to load the above url.
hmmmm.
I must have some other changes in my local tree.
I'm not seeing the crash.
-pn
Quick question:
does the crash happen WHEN the page is loaded or
when you move off of the page? Is any part of the image displayed?
ok. so that's 2 questions. But they are both quick.
-pn
Comment 8•25 years ago
|
||
The page loads first without the image being display. The image then is being
drawn (top to bottom) in the window. The crash occurs when the image is 3/4 point
of being drawn. I hope this helps you.
ok. I can finally see a crash. on linux...but not on NT.
onward
-pn
Updated•25 years ago
|
Whiteboard: could be in gfx?
Comment 10•25 years ago
|
||
-> m10
Assignee | ||
Comment 11•25 years ago
|
||
I have a fix for this. Waiting for tree to open for m10.
(note to self: if.cpp, ipng.cpp, nsPNGDecoder.cpp]
-pn
Status: REOPENED → RESOLVED
Closed: 25 years ago → 25 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 12•25 years ago
|
||
I checked in a fix for this.
The imglib displays the image until it hits the error
in the corrupted file. It should handle the error by
displaying the images' file name.
Closing bug as fixed so it gets into the testing cycle.
-pn
Assignee | ||
Comment 13•25 years ago
|
||
*** Bug 9000 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 14•25 years ago
|
||
*** Bug 9478 has been marked as a duplicate of this bug. ***
Comment 15•25 years ago
|
||
Pam, the crash no longer occurs in the Aug 24 build. After the image loads, it is
replaced with url text. Is this what I should expect ?
Assignee | ||
Comment 16•25 years ago
|
||
yep. The image displays until it hits the
corrupted data. Then it displays the broken/error
image info, which appears to be the filename these
days. I'm not sure who decided on the change....may
be the layout guys.
-pn
Updated•25 years ago
|
Status: RESOLVED → VERIFIED
Comment 17•25 years ago
|
||
Fixed in the August 24th Build.
You need to log in
before you can comment on or make changes to this bug.
Description
•