235832 - Crash on Save Image As with jpeg images and Save Page As on local files

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect)

Description

[Roscoe Primrose]

User-Agent:       
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7b) Gecko/20040225

Right-clicking and selecting save image as on jpeg images causes crash.

Reproducible: Always
Steps to Reproduce:
1.Go to a webpage that has jpeg images
2.Right-click to save image as
3.Crash

Actual Results:  
Mozilla crashed

Expected Results:  
Should have popped up a save location dialog and then saved image.

The URL above is just so the field isn't blank.  This has happenned on every
sight I've tried it with.

Comment 1

[Roscoe Primrose]

Still seeing with 2004022705

Comment 2

[Boris Zbarsky [:bzbarsky]]

Worksforme with a current linux build.  Is this OSX only?  What's the MacsBug stack?

Comment 3

[Stefan [:stefanh]]

Attachment: Mac OS X crash log

I can confirm this with Mozilla 2004022705 on Mac OS 10.2.8.

It's not just jpeg images... As far as I can tell, trying to save any image
with Save image as... will cause a crash.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Uh-oh.   Top of stack is:

 #0   0x01b1e154 in __dynamic_cast
 #1   0x01b14d28 in nsOSHelperAppService::GetMIMEInfoFromOS(char const*, char
const*, int*)
 #2   0x01b111f8 in nsExternalHelperAppService::GetFromTypeAndExtension(char
const*, char const*, nsIMIMEInfo**)
 #3   0x01b14a60 in nsOSHelperAppService::GetFromTypeAndExtension(char const*,
char const*, nsIMIMEInfo**)

I wonder how dynamic_cast manages to crash....

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

hm, that's no good
roscoe: could you generate an NSPR log, like this:
setenv NSPR_LOG_MODULES HelperAppService:5
setenv NSPR_LOG_FILE ~/crash.log
cd Desktop/Mozilla.app/Contents/MacOS
./mozilla

then crash, then attach ~/crash.log to this bug?

Comment 6

[Stefan [:stefanh]]

This is what I get (trying to save http://www.mozilla.org/images/mlogo.gif)

-1610609172[bcb80]: Getting mimeinfo from type 'image/gif' ext 'gif'
-1610609172[bcb80]: Mac: HelperAppService lookup for type 'image/gif' ext 'gif'
(IC: 0x4a84d60)
-1610609172[bcb80]: OS gave us: By Type: 0x538da80 By Ext: 0x54fe0f0 type has
default: false

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

oh, hm, doesn't .get() on a nsCOMPtr give me some nsDerivedSafe thingy?
let me suggest a patch...

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

Comment 9

[Boris Zbarsky [:bzbarsky]]

So... to make it _really_ clear to the compiler what's going on, I'd .get() and
NS_STATIC_CAST that to nsIMIMEInfo*....

Comment 10

[Dave Miller [:justdave]]

Confirming bug, I can duplicate this crash.

ok, tested the patch here, and it has absolutely no effect.  It still crashes,
and the stack trace looks exactly the same.  biesi had me try it again with
s/dynamic_cast/static_cast/g and that *did* work (the crash doesn't happen, and
the file successfully saves).

Comment 11

[Wayne Woods]

See also bug 235900.... the problem also exists when you try to "Save Page As"
and the open page is a local file. I haven't duped that bug yet, in case the two
save issues have different causes, but it seems a big coincidence. It looks like
the problem began with bug 234916, or right after it was fixed.

Comment 12

[Dave Miller [:justdave]]

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

Comment 13

[Dave Miller [:justdave]]

fixing summary to reflect the bug that was just duped here.  Biesi's patch
(modified as discussed on irc) fixes both issues.

Comment 14

[Boris Zbarsky [:bzbarsky]]

bug 234916 is totally unrelated to this problem.

Comment 15

[Hiro]

Attachment: firefox-bin.crash.log

It crashed, although the patch of 142479 was applied.
It checked by firefox of static build.
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; ja-JP; rv:1.7b) Gecko/20040229
Firefox/0.8.0+

Comment 16

[Jo Hermans]

And another confirm with build 20040228 on Mac OS X 10.2.8. Same stack.

I wasn't trying to confirm the bug, I've just met it myself, and I found this
one when I wanted to report it. 

Comment 17

[Boris Zbarsky [:bzbarsky]]

Guys, we know it crashes... no need to tell us over and over again.

Comment 18

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

well then, let's use static_cast...

I do not like this at all, but I guess I have to...

Comment 19

[Hiro]

additional comment:

It did not crash by Camino of created dynamic link build, without applying a patch.

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; ja-JP; rv:1.7b) Gecko/20040228
Camino/0.7+

Comment 20

[Hiro]

(In reply to comment #18)
> Created an attachment (id=142553)
> patch

It becomes a compile error.

sOSHelperAppService.pp ./mac/nsOSHelperAppService.cpp
mac/nsOSHelperAppService.cpp: In member function `virtual 
   already_AddRefed<nsIMIMEInfo> nsOSHelperAppService::GetMIMEInfoFromOS(const 
   char*, const char*, PRBool*)':
mac/nsOSHelperAppService.cpp:226: error: invalid static_cast from type `
   nsDerivedSafe<nsIMIMEInfo>*' to type `nsMIMEInfoBase*'
mac/nsOSHelperAppService.cpp:227: error: invalid static_cast from type `
   nsDerivedSafe<nsIMIMEInfo>*' to type `nsMIMEInfoBase*'
make[4]: *** [nsOSHelperAppService.o] Error 1

Comment 21

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch v2

bah, I hate nsDerivedSafe

try this instead

Comment 22

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 142566 [details] [diff] [review]
patch v2

r=me

Comment 23

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: another try

actually... maybe dynamic_cast has problems trying to cast to an abstract
class? this patch casts to nsMIMEInfoMac instead, which is a concrete class.
Maybe this is sufficient; I definitely prefer it to the static_cast patch

Comment 24

[doeni]

I am seeing a similar crash on Linux

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7b) Gecko/20040227

Stack trace :
#0  0x40706c31 in kill () from /lib/libc.so.6
#1  0x400e581d in pthread_kill () from /lib/libpthread.so.0
#2  0x400e5b9b in raise () from /lib/libpthread.so.0
#3  0x4149cff8 in NSGetModule ()
   from /usr/local/mozilla/components/libprofile.so
#4  0x400e9dfd in __pthread_sighandler () from /lib/libpthread.so.0
#5  <signal handler called>
#6  0x4163a6bb in nsGNOMERegistry::GetFromType(char const*) ()
   from /usr/local/mozilla/components/libdocshell.so
#7  0x41639016 in nsOSHelperAppService::GetFromType(char const*) ()
   from /usr/local/mozilla/components/libdocshell.so
#8  0x416395f8 in nsOSHelperAppService::GetMIMEInfoFromOS(char const*, char
const*, int*) () from /usr/local/mozilla/components/libdocshell.so
#9  0x4162f68d in nsExternalHelperAppService::GetFromTypeAndExtension(char
const*, char const*, nsIMIMEInfo**) ()
   from /usr/local/mozilla/components/libdocshell.so
#10 0x40a95797 in XPTC_InvokeByIndex () from /usr/local/mozilla/libxpcom.so
...

Build options :
--enable-default-toolkit=gtk2 --enable-calendar --enable-xft
--disable-postscript --disable-xprint --enable-crypto --disable-installer
--disable-debug --enable-optimize=-O3

Comment 25

[Christian :Biesinger (don't email me, ping me on IRC)]

[e-mail], you are seeing an entirely different crash, as the stack trace
clearly shows. Your crash is bug 235201

Comment 26

[Darin Fisher]

Comment on attachment 142566 [details] [diff] [review]
patch v2

sr=darin

this patch is fine by me.  however, if you want to try the other dynamic_cast
patch... ok.

Comment 27

[Christian :Biesinger (don't email me, ping me on IRC)]

I checked attachment 142566 [details] [diff] [review] in ("patch v2"). I'm leaving this bug open, because
I do prefer attachment 142587 [details] [diff] [review], but I'll need to find someone to test that patch.

Comment 28

[Zach Lipton [:zach]]

Patch v2 tested on OS 10.3 with a CVS tip pull and it appears to work fine. 

Comment 29

[Boris Zbarsky [:bzbarsky]]

Zach, could you test "another try" too?

Comment 30

[Zach Lipton [:zach]]

erm, sorry. What I tested was actually "another try," and it in fact does work. 

Sorry for the confusion. 

Comment 31

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: use dynamic_cast, updated to trunk

excellent!
thanks for the testing zach!

Comment 32

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 145101 [details] [diff] [review]
use dynamic_cast, updated to trunk

bz, could you review?

(darin gave sr in comment 26)

Comment 33

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 145101 [details] [diff] [review]
use dynamic_cast, updated to trunk

r=bzbarsky

Comment 34

[Christian :Biesinger (don't email me, ping me on IRC)]

latest patch checked in.

Checking in nsOSHelperAppService.cpp;
/cvsroot/mozilla/uriloader/exthandler/mac/nsOSHelperAppService.cpp,v  <-- 
nsOSHelperAppService.cpp
new revision: 1.34; previous revision: 1.33
done

Comment 35

[Christian :Biesinger (don't email me, ping me on IRC)]

it seems the last patch here caused crashes again, unfortunately. I backed it
out. (see bug 240748, bug Bug 240771)