GNOME Bugzilla – Bug 605480
No confirmation when overwriting files!
Last modified: 2010-01-27 10:12:01 UTC
In 2.29.3, when saving a page or image to disk, epiphany does not warn the user that the name they entered matches an already-existing file. Instead it overwrites the previously existing file without warning!
Confirming. Without looking at the code I think the patch will be something like this: gtk_file_chooser_set_do_overwrite_confirmation(GTK_FILE_CHOOSER(c), TRUE);
Created attachment 150841 [details] [review] Patch for ephy-file-chooser.c
Created attachment 150867 [details] [review] Confirm overwrite and Replace sub_resources. this patch adds confirmation for overwrite an existent file and save sub-resources the same way Firefox does.
Review of attachment 150867 [details] [review]: It's a good thing to have, but I think the patch needs work. ::: embed/ephy-web-view.c @@ +3408,3 @@ + g_object_unref (file); + return; + } But if it exists, shouldn't we then clean up the contents before saving the new one? I haven't checked, but IIRC this comes after we have a go from the file chooser, so if we get an exists error here, it means we already have a 'yes' from the use to overwrite, right? ::: lib/ephy-file-chooser.c @@ +371,3 @@ + gtk_file_chooser_set_do_overwrite_confirmation (GTK_FILE_CHOOSER (dialog), TRUE); + indentation is quite off here
Note that this replace/overwrite should be atomical, it should happen when the download finishes only. Not before. That's the logic we are following in downloads right now, see: bug #594192
>But if it exists, shouldn't we then clean up the contents before saving the new >one? On Firefox for instance, the existent resources folder is not cleaned up, instead, it does a merge with overwrite in the existent resources folder. With this patch, epiphany will have the same behavior. > I haven't checked, but IIRC this comes after we have a go from the file > chooser, so if we get an exists error here, it means we already have a 'yes' > from the use to overwrite, right? yes.
Created attachment 151339 [details] [review] Confirm overwrite and replace resources folder
> indentation is quite off here Fixed indentation on last patch submitted.
Review of attachment 151339 [details] [review]: Sorry for missing it the first time, one more cycle here. Do you have commit rights? ::: embed/ephy-web-view.c @@ +3402,2 @@ file = g_file_new_for_uri (destination_uri); + You added a space here, it seems. ::: lib/ephy-file-chooser.c @@ +374,1 @@ preview = gtk_image_new (); I just noticed that you are adding this to ephy-file-chooser.c; I suck, I'm sorry. I don't think changing the behavior for all dialogs is what we want here. We should actually apply this fix locally - this means you need to call this on the dialog that is created in src/window-commands.c, in function window_cm_file_save_as.
Created attachment 151842 [details] [review] Proposed patch for bug 60579 & bug 60480 Confirm overwrite + suggested filename.
(In reply to comment #10) > Created an attachment (id=151842) [details] [review] > Proposed patch for bug 60579 & bug 60480 > > Confirm overwrite + suggested filename. Please, don't =). One patch per issue!
Created attachment 151848 [details] [review] proposed patch.
Created attachment 151850 [details] [review] Proposed patch.
Review of attachment 151850 [details] [review]: Good, thanks!
Pushed f007602b631fee9ac23c909abb75bd606f501a4f to master.
I can't dig up the bug right now but I remember requests for making GMail handle mailto: links. Is that implementable with this fix?
(In reply to comment #16) > I can't dig up the bug right now but I remember requests for making GMail > handle mailto: links. Is that implementable with this fix? I think you're confusing bugs? No, this bug does not help that cause, anyway =).
hm yes that comment should have gone somewhere else...