Brian Savage Reporter
	Description • 23 years ago

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.5) Gecko/20011012
BuildID:    2001101202

When printing to a file, and if the file exists
there is not a question/dialog box asking if you would like
to overwrite the file or not, it just writes to that file


Reproducible: Always
Steps to Reproduce:
1.print to a file
2.open a new page
3.print to the file again

Actual Results:  the original file is destroyed and the second page
is printed to the file without question

Expected Results:  When printing to the same file a second time
mozilla should have asked me if I would like
to overwrite the existing file or not

	Boris Zbarsky [:bzbarsky]
	Comment 1 • 23 years ago

we should either not persist the filename or prompt on printing....

	Pete Zha Assignee
	Comment 2 • 23 years ago

taking

	Pete Zha Assignee
	Comment 3 • 23 years ago

We should deal this problem in different way in different situation.
1) If the file selected from "File picker", we should no ask user to confirm it
again since the user already confirmed in "File picker"
2) If the file name is inputed by user using keyboard, we should let him confirm.

	Pete Zha Assignee
	Comment 4 • 23 years ago

Confirm before replace file.

	rods (gone)
	Comment 5 • 23 years ago

Comment on attachment 91609 [details] [diff] [review]
patch

r=rods

	Boris Zbarsky [:bzbarsky]
	Comment 6 • 23 years ago

Comment on attachment 91609 [details] [diff] [review]
patch

>+    var promptService = Components.classes["@mozilla.org/embedcomp/prompt-service;1"].getService()
>+                        .QueryInterface(Components.interfaces.nsIPromptService)

Prefer:

var promptService =
Components.classes["@mozilla.org/embedcomp/prompt-service;1"]
		       .getService(Components.interfaces.nsIPromptService);

>+      var desc = gPrintSettings.toFileName + " " +
>+                 stringBundle.GetStringFromName("fileConfirm.exists") + "\n" +
>+                 stringBundle.GetStringFromName("fileConfirm.replace");

Please properly localize this mesage using formatStringFromName on the
stringbundle instead of doing this evil concatenation hack.  Please refrain
from sticking tha \n in there, too.

>+  <script type="application/x-javascript" src="chrome://inspector/content/jsutil/system/file.js"/>

This is unacceptable.  Inspector is an optional component; this code should not
depend on it being installed.  Please either have your own "file from string"
method or refactor the code to live somewhere where both inspector and this
code can use it.

This patch does not address the issue raised in comment 3, does it?

	Pete Zha Assignee
	Comment 7 • 23 years ago

More reseanable patch.

	Boris Zbarsky [:bzbarsky]
	Comment 8 • 23 years ago

Comment on attachment 91908 [details] [diff] [review]
patch (revise)

sr=bzbarsky if you use |foo != bar| instead of |! (foo == bar)|

	Pete Zha Assignee
	Comment 9 • 23 years ago

Should be fine. Thanks bzbarsky.

	Pete Zha Assignee
	Comment 10 • 23 years ago

Comment on attachment 91913 [details] [diff] [review]
patch

carry r&sr

	Scott Collins
	Comment 11 • 23 years ago

Comment on attachment 91913 [details] [diff] [review]
patch

a=scc for checkin to the mozilla trunk

	Brian Savage Reporter
	Comment 12 • 23 years ago

As a user I would prefer to always have it ask me to replace a file if it exists
even if it were choosen in the file picker

I wouldn't want silly mistakes made with the user interface to 
overwrite files which I want to keep

	Boris Zbarsky [:bzbarsky]
	Comment 13 • 23 years ago

When you choose the file in the filepicker, the filepicker _already_ prompts you
if the file exists.  So we would be asking the same question twice.  That would
get very frustrating very fast.

	Pete Zha Assignee
	Comment 14 • 23 years ago

checked in

	Mike Kaply [:mkaply]
	Comment 15 • 22 years ago

The print dialog wont close on OS/2 now.

Error in JS console:

Unrecognized path.

You can't just initialize an nsLocalFile with an empty path. You should be
checking the length of the path before initialization.

	Mike Kaply [:mkaply]
	Comment 16 • 22 years ago

Based on further investigation, I am reopening this.

The fix is definitely wrong. It makes a bad assumption.

The OS/2 and Windows versions of nsLocalFile require that InitWithNativePath
have nonempty string.

The code should have been made to handle this case.

I'm surprised the Unix nsLocalFile allows this. Maybe someone should fix it.

	Mike Kaply [:mkaply]
	Comment 17 • 22 years ago

First, only go down this path if printtofile is checked.

Second, if there is no filename, bail. You'll get the "empty file" error
message.

	Mike Kaply [:mkaply]
	Comment 18 • 22 years ago

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

	Boris Zbarsky [:bzbarsky]
	Comment 19 • 22 years ago

Comment on attachment 93165 [details] [diff] [review]
Better patch

sr=bzbarsky

	Pete Zha Assignee
	Comment 20 • 22 years ago

Comment on attachment 93165 [details] [diff] [review]
Better patch

>+      if (gPrintSettings.toFileName == "")
>         return false;
Do we need alert user here? Not just return false.

	Pete Zha Assignee
	Comment 21 • 22 years ago

Comment on attachment 93165 [details] [diff] [review]
Better patch

r=pete

	Mike Kaply [:mkaply]
	Comment 22 • 22 years ago

Pete,

I thought we would need to alert the user there explicitly, but it happens
somewhere else in the code.

	Asa Dotzler [:asa]
	Comment 23 • 22 years ago

Comment on attachment 93165 [details] [diff] [review]
Better patch

a=asa (on behalf of drivers) for checkin to 1.1

	Mike Kaply [:mkaply]
	Comment 24 • 22 years ago

Fixed again