488899 - Gmail "save image as" fails to identify/list MIME type correctly

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Peter Beckman]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090416 Shiretoko/3.5b4pre
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090416 Shiretoko/3.5b4pre

An email sent to Google GMAIL with a JPEG attachment is read, and the "View" link beside the image is clicked.  This opens a new tab, displays an address such as:

https://mail.google.com/mail/?ui=2&ik=1a2b3c4d5e6f&view=att&th=1a2b3c4d5e6f7a

The MIME type reported in "Page Info" is "image/jpeg", rendered in Quirks mode, ISO-8859-1 encoding, 66.68KB.

When you right click on the image and select "Save Image As..." the dialog box opens.  The two options given are:

Save As: "MS-DOS Executable" or "All Files"

Expected: "JPG Image" or "All Files"

The image is saved correctly and can be opened once saved to disk.  Also note that going to image files directly on a web server resulted in correct Mime type determination.  I'm not sure if this is a "Google is doing it wrong" 

Reproducible: Always

Steps to Reproduce:
1. Send an email to Google Gmail account with JPG attached.
2. Open email in Google Gmail.
3. Click "View"
4. Right-click on the image
5. Select "Save Image as..." from the context menu
6. In the "Save As" dialog box, the "Save As:" pulldown at the bottom of the dialog box displays "MS-DOS Executable" rather than the correct "JPG Image" for the mime type image/jpeg
Actual Results:  
The pulldown showed "MS-DOS Executable" rather than "JPG Image"

Expected Results:  
The pulldown should show "JPG Image" (or the correct mime type name).

Works in 3.0.8 for Google, so either a hack was removed or something got broken in 3.5b4pre.

Comment 1

[Peter Beckman]

Attachment: A screenshot of the Save Image dialog in Shiretoko with the MS-DOS Executable Save As: pulldown selected.

Comment 2

[Peter Beckman]

Attachment: Screenshot of correctly functioning Save Image dialog in Firefox 3.0.8

Comment 3

[Peter Beckman]

Some additional Information:
* Fresh Shiretoko Profile, no Addons
* Tried this with several JPG Images attached in Gmail, all failed the same way.
* Did NOT try this with other file formats.
* JPG files that have a direct URL (e.g. http://www.yahoo.com/logo.jpg) worked as expected.  This issue seems to be only with script-delivered images, and I could only recreate it in Gmail.

Comment 4

[Steven Michaud [:smichaud] (Retired)]

I can reproduce this with Firefox 3.0.8, yesterday's Minefield (trunk
aka mozilla-central) and Shiretoko (1.9.1 branch) nightlies, and even
with Firefox 2.0.0.20 -- but only on OS X (not on Windows or Linux).

Notice that the default file name in the Save As dialog (from step 6)
is 'mail.google.com' on OS X.  On Windows and Linux it's
'mail.google.com.jpeg'.  I'll bet this has something to do with the
problem.

It's difficult to say whose fault this is (Google's or Firefox's).
But I'll provisionally mark this a Cocoa Widgets bug, for further
investigation.

Comment 5

[Steven Michaud [:smichaud] (Retired)]

Peter:  The different behavior you saw on Firefox 3.0.8 may be due to
some browser setting.

Because I'm constantly trashing old profiles, I have very
plain-vanilla settings.

Comment 6

[Peter Beckman]

With a fresh profile, I too see the same behavior as reported.  I'm not entirely sure which plugin (or config setting?) might have resolved this or "fixed it."

I'm not sure the filename default has anything to do with it -- OS X tends to be "helpful" and remove file extensions and add them itself.  Though that's not to say there's not an OSX hack on Google's side.

I'll see if I can ngrep the conversation between google and the browser and post it here, then do the same with some other sites that do handle it correctly, and see if there is something they are doing differently or strangely.

Comment 7

[Peter Beckman]

My previous comment was vague -- I too see the same behavior as reported in 3.0.8 with a fresh profile -- the file type is MS-DOS Executable, rather than the expected JPEG Image.

Comment 8

[Steven Michaud [:smichaud] (Retired)]

I've now done a bit of debugging, and am quite sure this problem is
Google's fault.  In other words, it's Gmail's JavaScript code that
sets the default file name and the list of file types in the Save
Image dialog (the one that appears when you choose Save Image As from
the context menu that appears when you right-click on your email
message's attached image).

I found this out by setting breakpoints on (and adding logging to)
nsFilePicker::SetDefaultString() (which determines the default file
name) and nsFilePicker::AppendFilter() (which determines the list of
file types -- in this case "MS-DOS Executable" and "All files").

Both of these are (indirectly) called from
nsXULMenuCommandEvent::Run().  This means they're called from code in
the context menu that pops up when you right-click on the image.  The
contents of this menu are determined by Google (by Gmail-specific
JavaScript code).

This bug should either be marked INVALID or should be changed to a
Tech Evangelism bug (so that people from the Tech Evangelism team can
try to persuade Google to fix their code).

What do you think, Josh?

Comment 9

[Steven Michaud [:smichaud] (Retired)]

Further confirmation of my theory -- the string "MS-DOS Executable"
doesn't exist anywhere in the Mozilla tree.

Comment 10

[Steven Michaud [:smichaud] (Retired)]

If Gmail's context menu runs nsXULMenuCommandEvent::Run(), it must presumably be written in XUL.

Comment 11

[Steven Michaud [:smichaud] (Retired)]

(Following up comment #8)

> I've now done a bit of debugging, and am quite sure this problem is
> Google's fault.

I've now done a lot more debugging ... and no longer think so.

It's extraordinarily difficult to trace through combinations of
interpreted and compiled code.  But I'm now pretty sure the fault is
Firefox's, after all.

I'll have more to say on this tomorrow.

Comment 12

[Peter Beckman]

I'm interested in looking into the code, but as you know, jumping into a codebase as large as Mozilla's is daunting.  If you have any pointers in gh.mozilla.org to the code involved in the "Save Image as..." dialog box for the Mac side of things, I'd be happy to start looking myself.

Thanks for your continued efforts on this fairly minor issue Steven.

Comment 13

[Peter Beckman]

Headers from the request (IDs masked).  The headers were from Firefox 3.1b3, not from Shiretoko, though the request and response is assumedly the same.  I couldn't get Gmail to go HTTP on me, just HTTPS, even after turning that option off, so I had to use an addon to grab the headers.

https://mail.google.com/mail/?ui=2&ik=xxxxxxxx&view=att&th=xxxxxxxxxxxxxxxx&attid=0.1&disp=inline&zw

GET /mail/?ui=2&ik=xxxxxxxx&view=att&th=xxxxxxxxxxxxxxxx&attid=0.1&disp=inline&zw HTTP/1.1
Host: mail.google.com
User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3) Gecko/20090305 Firefox/3.1b3
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: en-us,en;q=0.5
Accept-Encoding: gzip,deflate
Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7
Keep-Alive: 300
Connection: keep-alive
Referer: https://mail.google.com/mail/?ui=2&view=bsp&ver=1qygpcgurkovy
Cookie: GX=xxxxxxxxxxxxxxxxxxxxxx; gmailchat=[e-mail]/xxxxxx; S=photos_html=xxxxxxx:gmail=xxxxxxxxx:gmproxy=xxxxxxxx:gmproxy_yj=xxxxxxxxxx; GMAIL_AT=xxxxxxxxxxxxxxxxxxxxxxxxxxx; PREF=ID=xxxxxxxxxxxxxxxx:U=xxxxxxxxxxxxxxxxxx:FF=4:LD=en:NR=100:TM=xxxxxxxxxxx:LM=xxxxxxxxxxxxxxxxx:GM=1:GBV=1:S=xxxxxxxxxxxxxxxxx; NID=22=xxxxxxxxxxxxxxxxxxxxxxxxxxxx; rememberme=true; SID=xxxxxxxxxxxxxxxxxxxxxxxxx; TZ=240; S=photos_html=xxxxxxxxxxxxxxxxxxxxxx

HTTP/1.x 200 OK
Cache-Control: private, max-age=86400
Expires: Thu, 23 Apr 2009 14:29:28 GMT
Date: Thu, 23 Apr 2009 14:29:28 GMT
Content-Type: image/jpeg
content-disposition: inline
Content-Length: 175181
X-Content-Type-Options: nosniff
Server: GFE/1.3

Comment 14

[Steven Michaud [:smichaud] (Retired)]

Attachment: Fix

Here's a patch for this bug.  A tryserver build will follow in a
while.

Turns out the context menu is Firefox's -- nsContextMenu in
browser/base/content/nsContextMenu.js.  Though Gmail may override
parts of it (it's hard for me to tell).

And the bug is buried very deep -- in Mac-specific code, though not in
Cocoa widget code.  What seems to have triggered it is that the image
file's URL (from Gmail) gives absolutely no clue what kind of file it
is, so getDefaultFileName() in toolkit/content/contentAreaUtils.js
falls back to using the host name ... which happens to end in '.com'.

Gmail gives Firefox the correct MIME-type information.  The bug is
that (in this case) Firefox allows the '.com' extension to override
it.  And '.com' (of course) is one of the allowed extensions for an
MS-DOS executable.  (I still don't know why the Firefox tree doesn't
contain the string "MS-DOS Executable' -- I suppose it must have
gotten the string from the OS.)

This bug wouldn't normally have been high on my priority list.  But
then I set out to clinch the case against Gmail ... and things turned
out rather differently :-)

Side note:  This kind of bug would be much easier to resolve if Google
apps didn't obfuscate their JavaScript code.

Comment 15

[Steven Michaud [:smichaud] (Retired)]

Attachment: Patch for debugging (*not* a fix)

(In reply to comment #12)

> I'm interested in looking into the code, but as you know, jumping
> into a codebase as large as Mozilla's is daunting.  If you have any
> pointers in gh.mozilla.org to the code involved in the "Save Image
> as..." dialog box for the Mac side of things, I'd be happy to start
> looking myself.

It's certainly not for the faint of heart.  But even a long journey
starts with a single step :-)

Here's a patch that shows you what I did to debug this particular bug.
It should give you an idea of the kind of work involved.  I started by
running a debug build of Firefox (actually an opt build with debug
symbols) in gdb, then breaking while this bug's save image dialog was
displayed -- to get a stack trace.  Then (after looking at the stack
trace) I added logging to widget/src/cocoa/nsFilePicker.mm and to
dom/src/events/nsJSEventListener.cpp.  The logging in
nsJSEventListener::HandleEvent() showed me that the following
JavaScript snippet gets invoked when you choose Save Image As from
this bug's context menu:

function oncommand(event) {
    gContextMenu.saveMedia();
}

I searched for 'gContextMenu.saveMedia()' in the Mozilla tree, found
it, and continued from there.

> Thanks for your continued efforts on this fairly minor issue Steven.

You're most welcome.

Comment 16

[Steven Michaud [:smichaud] (Retired)]

One thing I forgot:

In order to see output from JavaScript dump() in the System Console
(or at the Terminal command line), you need to add a
'browser.dom.window.dump.enabled' boolean setting (in about:config),
and set it to 'true'.

Comment 17

[Steven Michaud [:smichaud] (Retired)]

Here's the tryserver build for my patch in comment #14:

https://build.mozilla.org/tryserver-builds/2009-04-23_10:[e-mail]-bugzilla488899/[e-mail]

Comment 18

[Steven Michaud [:smichaud] (Retired)]

One more thing.  Here's the .mozconfig file I used to build my opt build with debug symbols:

export CFLAGS="-g -gfull"
export CXXFLAGS="-g -gfull"
. $topsrcdir/browser/config/mozconfig
mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/obj-firefox
mk_add_options MOZ_MAKE_FLAGS=-j4
mk_add_options AUTOCONF=autoconf213
ac_add_options --enable-optimize
ac_add_options --enable-tests
ac_add_options --enable-cpp-rtti
ac_add_options --with-macos-sdk=/Developer/SDKs/MacOSX10.4u.sdk

Comment 19

[Josh Aas]

Bug 489864 also fixes this bug, the internet config stuff is a mess and a common source of bugs, we should just remove it.

Comment 20

[Josh Aas]

Comment on attachment 374288 [details] [diff] [review]
Fix

I'm fine with this for a branch patch but on trunk I think the way to go is bug 489864. Please don't land this on trunk until we figure out if we're going to take the other patch.

Thanks for figuring this out!

Comment 21

[Steven Michaud [:smichaud] (Retired)]

> on trunk I think the way to go is bug 489864

I agree.  I'll try to review your patch today.

> Thanks for figuring this out!

You're most welcome.

Comment 22

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

This is FIXED now that bug 489864 has landed, right?

Comment 23

[Steven Michaud [:smichaud] (Retired)]

Yes.  I just checked in FF 3.6.3.  (Though the patch for bug 489864 landed on the trunk, this happened before the 1.9.2 branch was created.  So the 1.9.2 branch also has that patch.)