238464 - Crash setting wallpaper to a broken image

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Jon Henry]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7b) Gecko/20040323 Firefox/0.8.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7b) Gecko/20040323 Firefox/0.8.0+

If you right-click a broken image icon and select Set as Wallpaper, then click
Select in the subsequent dialog, Firefox will crash in
nsWindowsShellService::SetDesktopBackground on a null pointer. I have a patch
that seems to fix the problem. Maybe this action shouldn't even be enabled on
the context menu for non-existant images, but since the other options are (like
View Image), I'm just fixing the underlying crash.

Reproducible: Always
Steps to Reproduce:
1. Try to set your desktop wallpaper to a broken image icon.


Actual Results:  
Firefox will crash.

Expected Results:  
No crash, and no change in the wallpaper image either.

Comment 1

[Jon Henry]

Attachment: Patch

Easy one-line patch that fixes the crash.

Comment 2

[Jon Henry]

Comment on attachment 144631 [details] [diff] [review]
Patch

Could you please take a look at this when you get the chance, Ben?

Comment 3

[Jesse Ruderman]

WFM.  What am I supposed to click in the "Set Wallpaper" dialog after opening it?

Comment 4

[Jon Henry]

After the "set wallpaper" dialog shows up, clicking the "set wallpaper" button
in that dialog causes the crash for me. If it would be helpful I can post a
stack trace.

Comment 5

[timeless]

*** This bug has been marked as a duplicate of 136016 ***

Comment 6

[timeless]

Comment on attachment 144631 [details] [diff] [review]
Patch

>     rv = request->GetImage(getter_AddRefs(container));
>-    if (!request) return rv;
>+    if (!container) return rv;

rv happens to be NS_OK. Which means you return the wrong thing. I'm sorry no
one looked at this and that I ended up resolving your bug w/ patch as a dupe of
another one which just got a patch a moment ago. I only just started triaging
wallpaper bugs after i ran into the older report and analyzed the problem.

Comment 7

[Jon Henry]

Correct me if I'm wrong, but this is not a dupe. Yes, it's exactly the same
issue, but Firefox forked and then rewrote most of winhooks so the patch from
bug 136016 doesn't work. I'll attach a new patch that has the less bogus return
value from the other bug.

Comment 8

[Jon Henry]

Attachment: Better patch

The same fix as the other bug, but applied to the forked Firefox files. I
should have caught the bogus return value, that was a stupid oversight.

Comment 9

[timeless]

sorry, i missed that, the code looked the same.

Comment 10

[José Jeria]

*** Bug 243080 has been marked as a duplicate of this bug. ***

Comment 11

[Michael Lefevre]

Comment on attachment 147245 [details] [diff] [review]
Better patch

moving some easy reviews to mconnor who should be getting to them faster than
Ben

Comment 12

[Mike Connor [:mconnor]]

Comment on attachment 147245 [details] [diff] [review]
Better patch

sorry for the latency here, r=[e-mail]

Throwing into the approval-aviary bin.

Comment 13

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 147245 [details] [diff] [review]
Better patch

a=[e-mail]

Comment 14

[Jon Henry]

I don't mean to be a pain, but I would appreciate it if someone could check this
in for me now that it has approval since I do not have CVS access (mconnor,
Jesse?). Thanks!

Comment 15

[Jesse Ruderman]

I checked in Jon's fix, trunk and aviary.