Closed
Bug 601422
Opened 14 years ago
Closed 14 years ago
Crash [@ nsImageDocument::ShrinkToFit] in removed frame
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla2.0b7
People
(Reporter: jruderman, Assigned: MatsPalmgren_bugz)
Details
(Keywords: crash, testcase, Whiteboard: [sg:dos])
Crash Data
Attachments
(3 files, 1 obsolete file)
529 bytes,
text/html
|
Details | |
2.27 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
1.49 KB,
patch
|
jst
:
approval2.0+
|
Details | Diff | Splinter Review |
mImageContent is null.
Assignee | ||
Comment 1•14 years ago
|
||
The other nsIImageDocument methods have the same problem.
Assignee: nobody → matspal
Attachment #480464 -
Attachment is obsolete: true
Assignee | ||
Updated•14 years ago
|
OS: Mac OS X → All
Hardware: x86 → All
Whiteboard: [sg:dos]
Assignee | ||
Comment 2•14 years ago
|
||
Add null-check and silent return.
Or should we return NS_ERROR_NOT_AVAILABLE perhaps?
Attachment #480510 -
Flags: review?(Olli.Pettay)
Comment 3•14 years ago
|
||
Comment on attachment 480510 [details] [diff] [review]
Patch rev. 1
> NS_IMETHODIMP
> nsImageDocument::RestoreImageTo(PRInt32 aX, PRInt32 aY)
> {
>+ if (!mImageContent) {
>+ return NS_OK;
>+ }
> return ScrollImageTo(aX, aY, PR_TRUE);
> }
This change shouldn't be needed, since
ScrollImageTo calls RestoreImage which you make
null safe.
> NS_IMETHODIMP
> nsImageDocument::ToggleImageSize()
> {
>+ if (!mImageContent) {
>+ return NS_OK;
>+ }
> mShouldResize = PR_TRUE;
> if (mImageIsResized) {
> mShouldResize = PR_FALSE;
> ResetZoomLevel();
> RestoreImage();
> }
> else if (mImageIsOverflowing) {
> ResetZoomLevel();
And I think this isn't needed either.
Attachment #480510 -
Flags: review?(Olli.Pettay) → review+
Assignee | ||
Comment 4•14 years ago
|
||
Fair enough. I think the former is slightly more robust, but
the testcase should catch a future change in ScrollImageTo,
ToggleImageSize that would make them crash in this case.
Assignee | ||
Comment 5•14 years ago
|
||
Comment on attachment 480615 [details] [diff] [review]
Patch rev. 2 (nits fixed)
Trivial fix for a null-pointer crash.
Attachment #480615 -
Flags: approval2.0?
Updated•14 years ago
|
Attachment #480615 -
Flags: approval2.0? → approval2.0+
Assignee | ||
Comment 6•14 years ago
|
||
Status: NEW → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Updated•14 years ago
|
Target Milestone: mozilla2.0b8 → mozilla2.0b7
Updated•14 years ago
|
Crash Signature: [@ nsImageDocument::ShrinkToFit]
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•