341218 - GTK2 filepicker "confirm overwrite" should use native GTK2 API when available

Status: VERIFIED FIXED

(Core :: Widget: Gtk, defect)

Description

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

Right now, if you do a "Save As" and choose an existing file, we ask "Are you sure (yes/no)" (or words to that effect). If you choose "no", we don't take you back to the dialog, you have to start the operation again.

This is because we're rolling our own confirmation dialog and we only show it after the filepicker is done. Instead we should just use the built-in GTK2 overwrite confirmation.

Comment 1

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

Attachment: fix

Comment 2

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

could we take the user back to the dialog even when this API does not exist?

Comment 3

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

I suppose we could. However, I do not care.

Comment 4

[Brian Ryner (not reading)]

Comment on attachment 225248 [details] [diff] [review]
fix

>--- widget/src/gtk2/nsFilePicker.cpp	17 Aug 2005 13:41:44 -0000	1.11
>+++ widget/src/gtk2/nsFilePicker.cpp	12 Jun 2006 08:02:15 -0000
>@@ -555,43 +558,51 @@ nsFilePicker::Show(PRInt16 *aReturn)
>       const char *filter_pattern = mFilters[i]->get();
>       _gtk_file_filter_set_name (filter, filter_pattern);
>     }
> 
>     _gtk_file_chooser_add_filter (GTK_FILE_CHOOSER (file_chooser), filter);
> 
>     // Set the initially selected filter
>     if (mSelectedType == i) {
>       _gtk_file_chooser_set_filter (GTK_FILE_CHOOSER(file_chooser), filter);
>     }
>   }
> 
>+  PRBool manualConfirm = PR_TRUE;

I'm not quite sure about the naming of this variable, it seems more like it's "autoConfirm" or "builtinConfirm".

In any case, r+sr=me

Comment 5

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

I'm making it checkForOverwrite.

Comment 6

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

Checked in.

Comment 7

[Magnus Melin [:mkmelin]]

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

Comment 8

[Wolfgang Rosenauer [:wolfiR]]

Comment on attachment 225248 [details] [diff] [review]
fix

We should consider this (together with the fix in bug 349790) for Firefox 2.

Comment 9

[Mike Schroepfer]

Comment on attachment 225248 [details] [diff] [review]
fix

a=schrep for drivers.

Comment 10

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

Checked into 1.8.1 branch

Comment 11

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 225248 [details] [diff] [review]
fix

Distros should probably get this fix.

Comment 12

[Jay Patel [:jay]]

Comment on attachment 225248 [details] [diff] [review]
fix

Approved for 1.8.0 branch, a=jay for drivers.

Comment 13

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

checked into 1.8.0 branch

Comment 14

[Adam Guthrie]

Verified FIXED using the steps outlined in comment 0 using the following builds:
* Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.9pre) Gecko/20061201 Firefox/1.5.0.9pre
* Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.1pre) Gecko/20061201 BonEcho/2.0.0.1pre