Closed Bug 459424 Opened 11 years ago Closed 11 years ago

[FIX]Null-deref crash when zooming on page with invalid external resource documents

Categories

(Core :: DOM: Core & HTML, defect, critical)

defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Details

Attachments

(2 files)

We pass null documents to the enumerator in that situation.  Fix coming up.  Jesse, do we have a good way of crashtesting something like this?
Attached patch FixSplinter Review
Attachment #342625 - Flags: superreview?(roc)
Attachment #342625 - Flags: review?(roc)
Jesse, see comment 0?
Attached file Test page
There are a few existing automated tests that mess with zoom, such as these tests that use nsIMarkupDocumentViewer.fullZoom:

http://mxr.mozilla.org/mozilla-central/search?string=fullzoom&find=test

I think you're supposed to use a mochitest to ensure privileges are available.
Attachment #342625 - Flags: superreview?(roc)
Attachment #342625 - Flags: superreview+
Attachment #342625 - Flags: review?(roc)
Attachment #342625 - Flags: review+
Comment on attachment 342625 [details] [diff] [review]
Fix

+  PRBool next =
+    aData->mDocument ? args->callback(aData->mDocument, args->data) : PR_TRUE;

aData->mDocument && args->callback(...)
I'm going to add zoom support to reftests in bug 458487 (or maybe in a subordinate bug spun off from that).
> aData->mDocument && args->callback(...)

You mean:

  !aData->mDocument || args->callback(...);

?  I can do that I guess; but do you really think it's more readable?
Pushed changeset b5b7b5523c66 with test.
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
This was backed out as it seemed the most likely cause of multiple unit test failures.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
The backout did, in fact, clear up the unit test failures.
Checked back in, with a change to the test to reset zoom back to 1 to fix the failures.
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Component: Content → DOM
QA Contact: content → general
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.