354954 - [Cocoa] "Save Page As..." doesn't work.

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Hiro]

In Cocoa widget Firefox, "Save Page As..." doesn't work. 

Mac OS X 10.3.9

O.K
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060926 Minefield/3.0a1

N.G
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060929 Minefield/3.0a1

Comment 1

[Jesse Ruderman]

When I press Cmd+S, I get a dialog that doesn't let me choose between "HTML only" and "Complete web page".  I'm scared to find out what happens if I click the "Save" button in the dialog.

Mac trunk debug.

Comment 2

[Hiro]

(In reply to comment #1)
> When I press Cmd+S, I get a dialog that doesn't let me choose between "HTML
> only" and "Complete web page".  I'm scared to find out what happens if I click
> the "Save" button in the dialog.

Yes, The dialog without "HTML only" and "Complete web page" comes out when 
"Save Page As..." is chosen from the File menu. 
And anything doesn't happen even if "Save" button is pushed. 

Comment 3

[Karsten Düsterloh]

This is broken in current SeaMonkey trunk Mac builds also. :(

Comment 4

[Simon Howes]

I can confirm, I get this also.

Comment 5

[Phil Ringnalda (:philor)]

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

Comment 6

[Phil Ringnalda (:philor)]

Both Save Page As... (File menu or context menu) and Save Link As... fail to
save the page, dropping a

Error: uncaught exception: [Exception... "Component returned failure code:
0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIPrefBranch.setComplexValue]" 
nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)"  location: "JS frame ::
chrome://global/content/contentAreaUtils.js :: getTargetFile :: line 545" 
data: no]

in the Error Console.

Saving files from the unknown content type dialog works fine as long as you
have the pref set to not ask where to save, but if you have the filepicker come
up, then you get the randomly-named temp file which doesn't get renamed or
moved to what you chose in the filepicker, and you get the same Error Console
message.

Comment 7

[Phil Ringnalda (:philor)]

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

Comment 8

[Josh Aas]

Attachment: fix v0.8

This patch fixes things. A comment above the file save code that explains why this is all busted:

-// Note: I don't think Chimera is using this at all.  So - it hasn't been tested.
-//       If you end up calling it, let me know how it turns out.

This patch still needs a localizable "Save as:" string. I'm just posting v0.8 now for safe keeping, I'll try to finish it up tomorrow.

Comment 9

[Josh Aas]

Attachment: fix v1.0

Comment 10

[Stefan [:stefanh]]

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

Comment 11

[dales]

(In reply to comment #10)
> *** Bug 356710 has been marked as a duplicate of this bug. ***
> 

I agree with you now ... I've applied Josh's 1.0 patch to my fresh source pulled this morning about 9:30 am EDT and I can do Save As ... for both saving a page and for downloading a file.  I've only done a little testing on this build so far, but now I'm looking forward to seeing this approved and checked in.

There is still a cocoa toolkit problem with the Application menu in SeaMonkey trunk ... it's missing 3 - 4 items (all the Hide commands & ...?).  I'm looking for a related bug ... no likelies in sight yet.

     Dale

Comment 12

[Stefan [:stefanh]]

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

Comment 13

[Stuart Morgan]

Comment on attachment 242377 [details] [diff] [review]
fix v1.0

I haven't tested, but the code looks good.  r=me.

A couple thoughts that you can ignore or not as you see fit:

>+#define ACCESSORY_VIEW_PADDING 5
>+#define SAVE_TYPE_CONTROL_TAG 1

Unless this is the moz standard, you might want to use consts instead of #defines, like we do in Camino.  It makes like much easier in the debugger.
 
>+  for (int i = 0; i < numMenuItems; i++) {
>+    const nsString& currentTitle = *mTitles[i];
>+    NSString *titleString = [[[NSString alloc] initWithCharacters:currentTitle.get()
>+                                                           length:currentTitle.Length()] autorelease];
>+    [popupButton addItemWithTitle:titleString];
>+  }

I don't have any idea how big numMenuItems can be, but if it's ever more than a handful you might want to explicitly release titleString instead of autoreleasing it (since it's used in such a confined location, and thus is easy to do safely).

>+  float popupOriginX = [textField frame].size.width + ACCESSORY_VIEW_PADDING * 2;

This would be a bit less fragile as
NSMaxX([textField frame]) + ACCESSORY_VIEW_PADDING

Comment 14

[Josh Aas]

I will make most if not all of the changes that smorgan suggested on checkin.

Comment 15

[Josh Aas]

fixed on trunk

Comment 16

[Adam Guthrie]

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