Closed Bug 604262 Opened 10 years ago Closed 10 years ago

Crash [@ nsImageLoadingContent::OnStartDecode]


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

Not set



Tracking Status
blocking2.0 --- final+
blocking1.9.2 --- .17+
status1.9.2 --- .17-fixed
blocking1.9.1 --- .19+
status1.9.1 --- .19-fixed


(Reporter: jruderman, Assigned: jst)



(Keywords: crash, regression, testcase, Whiteboard: [softblocker][fx4-fixed-bugday])

Crash Data


(6 files)

No description provided.
It looks like this does not crash on Firefox 3.6. Should that be a blocker?
blocking2.0: --- → ?
Keywords: regression
OS: Mac OS X → All
Hardware: x86 → All
Version: unspecified → Trunk
Attached patch Fix.Splinter Review
This makes it so that only chrome can call methods on the interfaces that are not explicitly listed in classinfo for input elements.
Attachment #483355 - Flags: review?(jonas)
blocking2.0: ? → final+
Comment on attachment 483355 [details] [diff] [review]

If we're able to restrict access to Components.interfaces for FF4, then we should back this out.

r=me for now though.
Attachment #483355 - Flags: review?(jonas) → review+
Assignee: nobody → jst
Bug 605271 isn't going to make it.  We should land this on trunk.
Keywords: checkin-needed
I was thinking to push this but it doesn't pass try and fails a lot of tests. One of them is content/base/test/test_copyimage.html

For more information, you can see the try results here:
Keywords: checkin-needed
Whiteboard: [dont pass try]
Whiteboard: [dont pass try] → [dont pass try], softblocker
Whiteboard: [dont pass try], softblocker → [dont pass try][softblocker]
So this got nasty. What happens here is that we have a bunch of code that ends up running from within untrusted JS, and that code calls into the image code to do whatever, and we check to see if the caller is trusted, but we find the untrusted JS code, and deny the C++ code from doing what it needs to do. This patch adds the (in)appropriate nsCxPusher's to push a null context where needed to hide the fact that this code was called from untrusted JS so that the C++ code can do what it needs to do. This really sucks, but I don't think we have a better option here.
Attachment #507741 - Flags: review?(jonas)
Comment on attachment 507741 [details] [diff] [review]
Fix test failures.

>@@ -145,7 +145,13 @@ nsSVGImageFrame::~nsSVGImageFrame()
>   if (mListener) {
>     nsCOMPtr<nsIImageLoadingContent> imageLoader = do_QueryInterface(mContent);
>     if (imageLoader) {
>-      imageLoader->RemoveObserver(mListener);
>+      // Push a null JSContext on the stack so that code that runs
>+      // within the below code doesn't think it's being called by
>+      // JS. See bug 604262.
>+      nsCxPusher pusher;
>+      pusher.PushNull();
>+     imageLoader->RemoveObserver(mListener);

There's a whitespace issue on the last line here.

r=me with that fixed.
Attachment #507741 - Flags: review?(jonas) → review+
This patch landed as:
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b11

Mozilla/5.0 (Windows NT 5.1; rv:2.0b11) Gecko/20100101 Firefox/4.0b11 ID:20110203141415

does not crash when loading attachment.

4.0b10 did crash ! So it seems problem is solved.
Verified on Linux, too — with Mozilla/5.0 (X11; Linux i686; rv:2.0b12pre) Gecko/20110204 Firefox/4.0b12pre  (crashed with 20110124030332)
Whiteboard: [dont pass try][softblocker] → [dont pass try][softblocker][bugday0204]
The patch landed, just looks like the bug was never closed.  Thanks for catching that Ben and Aleksej!
Whiteboard: [dont pass try][softblocker][bugday0204] → [softblocker][bugday0204]
Whiteboard: [softblocker][bugday0204] → [softblocker][fx4-fixed-bugday]
blocking1.9.2: --- → .15+
blocking1.9.1: --- → .18+
Deferring to a future point release as we have run out of time. If this absolutely needs to be fixed and can land today or tomorrow, please send a note to
blocking1.9.1: .19+ → .20+
blocking1.9.2: .17+ → .18+
We still do need this one
blocking1.9.1: .20+ → .19+
blocking1.9.2: .18+ → .17+
This landing seems to kill JS "var ButtonImage = new Array()" function.
Sorry for my insufficient explanation.
After changes pushed with changeset a7a241b581ad, 'onmouseover' and 'onmouseout' function don't work.
Can you file a new bug with a testcase and CC me?
This turned the 1.9.2 reftest tinderboxen orange because of sync/async differences between trunk and branch. This patch should make them go green. It has r=bz and a=dveditz (both in person).
Attachment #525442 - Flags: review+
Attached patch Fix for 1.9.1Splinter Review
This is a combined patch against 1.9.1 of all that was checked in for 1.9.2. I found out that nsCxPusher::PushNull doesn't exist on the 1.9.1 branch, and adding it isn't trivial, so I did the pushing and popping by hand instead. And some of the changed code didn't exist on 1.9.1 so there are a couple of hunks that are different here.
Attachment #526410 - Flags: review?(mrbkap)
(In reply to comment #20)
> Can you file a new bug with a testcase and CC me?

bug 649326 was filed on this, but was fixed by the patch in comment 21.
Comment on attachment 526410 [details] [diff] [review]
Fix for 1.9.1

Looks good. I was wondering if it might have been worth it to add a nsNullCxPusher class, but it only helps one case added in this patch, so probably not.
Attachment #526410 - Flags: review?(mrbkap) → review+
Attachment #526410 - Flags: approval1.9.1.20?
Attachment #526410 - Flags: approval1.9.1.19?
Comment on attachment 526410 [details] [diff] [review]
Fix for 1.9.1

Approved for, a=dveditz for release-drivers

Please land ASAP so we can re-spin today.
Attachment #526410 - Flags: approval1.9.1.20?
Attachment #526410 - Flags: approval1.9.1.19?
Attachment #526410 - Flags: approval1.9.1.19+
I checked this with the attached testcase in build 1 (before the fix on 4/20) of I do not see a crash. This is on Windows 7.

How does one cause the crash?
(It won't crash, but it should be enough to verify the fix here.)
Crash Signature: [@ nsImageLoadingContent::OnStartDecode]
Blocks: 691785
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.