Closed Bug 235832 Opened 20 years ago Closed 20 years ago

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

Categories

(Core Graveyard :: File Handling, defect)

PowerPC
macOS
defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: roscoe, Assigned: Biesinger)

References

()

Details

Attachments

(4 files, 3 obsolete files)

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.
Still seeing with 2004022705
Worksforme with a current linux build.  Is this OSX only?  What's the MacsBug stack?
Attached file 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.
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....
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?
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
oh, hm, doesn't .get() on a nsCOMPtr give me some nsDerivedSafe thingy?
let me suggest a patch...
Attached patch patch (obsolete) — Splinter Review
So... to make it _really_ clear to the compiler what's going on, I'd .get() and
NS_STATIC_CAST that to nsIMIMEInfo*....
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).
Assignee: general → file-handling
Status: UNCONFIRMED → NEW
Component: Browser-General → File Handling
Ever confirmed: true
QA Contact: general → ian
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.
*** Bug 235900 has been marked as a duplicate of this bug. ***
fixing summary to reflect the bug that was just duped here.  Biesi's patch
(modified as discussed on irc) fixes both issues.
Summary: Crash on Save Image As with jpeg images → Crash on Save Image As with jpeg images and Save Page As on local files
bug 234916 is totally unrelated to this problem.
Attached file 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+
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. 
Guys, we know it crashes... no need to tell us over and over again.
Attached patch patch (obsolete) — Splinter Review
well then, let's use static_cast...

I do not like this at all, but I guess I have to...
Assignee: file-handling → cbiesinger
Attachment #142479 - Attachment is obsolete: true
Status: NEW → ASSIGNED
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+
(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
Attached patch patch v2Splinter Review
bah, I hate nsDerivedSafe

try this instead
Attachment #142553 - Attachment is obsolete: true
Attachment #142566 - Flags: superreview?(darin)
Attachment #142566 - Flags: review?(bzbarsky)
Comment on attachment 142566 [details] [diff] [review]
patch v2

r=me
Attachment #142566 - Flags: review?(bzbarsky) → review+
Attached patch another try (obsolete) — Splinter Review
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
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
doeni@evhr.net, you are seeing an entirely different crash, as the stack trace
clearly shows. Your crash is bug 235201
Blocks: 236017
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.
Attachment #142566 - Flags: superreview?(darin) → superreview+
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.
Blocks: 236149
Patch v2 tested on OS 10.3 with a CVS tip pull and it appears to work fine. 
Zach, could you test "another try" too?
erm, sorry. What I tested was actually "another try," and it in fact does work. 

Sorry for the confusion. 
excellent!
thanks for the testing zach!
Attachment #142587 - Attachment is obsolete: true
Comment on attachment 145101 [details] [diff] [review]
use dynamic_cast, updated to trunk

bz, could you review?

(darin gave sr in comment 26)
Attachment #145101 - Flags: review?(bzbarsky)
Comment on attachment 145101 [details] [diff] [review]
use dynamic_cast, updated to trunk

r=bzbarsky
Attachment #145101 - Flags: review?(bzbarsky) → review+
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
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
it seems the last patch here caused crashes again, unfortunately. I backed it
out. (see bug 240748, bug Bug 240771)
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: