417910 - Save As panel doesn't remember last saving format

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect, P2)

Description

[Dominik Madon]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3) Gecko/2008020511 Firefox/3.0b3
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3) Gecko/2008020511 Firefox/3.0b3

Since version 3, the saving format selector is not set to last selection.

Reproducible: Always

Steps to Reproduce:
1. Open the "Save As" panel on a random page.
2. Change the format to "Web Page, HTML only"
3. Save the page
4. Open the "Save As" panel.
Actual Results:  
The format selector is set on "Web Page, complete"

Expected Results:  
The format should be set on "Web Page, HTML only"

I'm using firefox 3.0b3.

Comment 1

[Marcia Knous [:marcia]]

Confirming - I see this using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b4pre) Gecko/2008021604 Minefield/3.0b4pre. I checked Firefox 2.0.0.12 and it does default to the last selection that you made.

Comment 3

[Carsten Book [:Tomcat]]

this seems to a be a regression since Firefox 2, since its also broken in the Alpha 1 -> Beta 3 Versions of Firefox 3.

Comment 4

[Josh Aas]

Attachment: fix v1.0

This should work at least as well as our carbon impl did.

Comment 5

[Josh Aas]

We can also just kill off the "sCurrentProcessSignature" variable, it isn't used.

Comment 6

[Stuart Morgan]

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

r=smorgan

>+#define FILE_TYPE_NATIVE_PREF @"GeckoFilePickerFormatIndex"

Why a #define, rather than a |static NSString* const|?

Comment 7

[Josh Aas]

Attachment: fix v1.1

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

so if someone created two filepickers, opened one and waited for it to close, then opened the other and waited for it to close, then asked for the filter index of the first one, they could get the wrong value? I.e., shouldn't we keep a per-filepicker filterIndex as well as store it in prefs?

Comment 9

[Josh Aas]

Attachment: fix v2.0

Uses gecko prefs only to init the index value for each instance, saves out to prefs whenever an index is used.

There is a bunch of comment cleanup in this patch, sorry but that style was driving me nuts and it didn't match the rest of cocoa widgets.

Comment 10

[Josh Aas]

Attachment: fix v2.1

Forgot a sanity check.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 306317 [details] [diff] [review]
fix v2.1

+const char* kLastTypeIndexPref = "filepicker.lastTypeIndex";

Infintesimally more efficient to write "const char kLastTypeIndexPref[] = ..."

Comment 12

[Josh Aas]

Comment on attachment 306317 [details] [diff] [review]
fix v2.1

This is low risk, reward is it fixes a blocker :) Doesn't necessarily need to make a beta, it would just be nice.

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 306317 [details] [diff] [review]
fix v2.1

a1.9b4=beltzner

Comment 14

[Josh Aas]

landed on trunk

Comment 15

[Marcia Knous [:marcia]]

verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b4pre) Gecko/2008022904 Minefield/3.0b4pre.