Closed Bug 356287 Opened 13 years ago Closed 13 years ago

Download fails if TMP environment variable is set using forward slashes

Categories

(Core :: XPCOM, defect, minor)

x86
OS/2
defect
Not set
minor

Tracking

()

VERIFIED FIXED

People

(Reporter: christian.hennecke, Assigned: mozilla)

Details

(Keywords: verified1.8.1.1)

Attachments

(1 file)

User-Agent:       Mozilla/5.0 (OS/2; U; Warp 4.5; de; rv:1.8.0.7) Gecko/20060910 Firefox/1.5.0.7
Build Identifier: Mozilla/5.0 (OS/2; U; Warp 4.5; de; rv:1.8.0.7) Gecko/20060910 Firefox/1.5.0.7

When the TMP environment variable is set in CONFIG.SYS using forward slashes as path seperators on an OS/2 system, downloading a file by clicking on a download link fails. This even happens when TEMP is set using backslashes.

On an OS/2 system, you can expect the TEMP, TMP, and TMPDIR variables to be set in CONFIG.SYS. TEMP is the "traditional" environment variable to point to the directory for temporary files and always uses backslashes, e.g., c:\tmp. However, the TMP and TMPDIR environment variables are also commonly used for software that has been ported from the Unix world. Most of these ported software packages *require* the use of forward slashes, i.e., SET TMP=c:/tmp. A cross-platform solution would be to change the way the path is interpreted or to check the validity of the variable and try using another if the check fails.

Reproducible: Always

Steps to Reproduce:
1. Go to a site that offers files for download
2. Click on a download link to download and save the file, e.g., for a ZIP file


Actual Results:  
The following error message is displayed in a dialog:

"Downloading

could not be saved, because an unknown error occurred.
Try saving to a different location."

Expected Results:  
It should have started downloading the file and should have displayed the download location selection dialog.
Yes, I see the same behavior. But a workaround is easy (use a command file or Run!'s *.env to reset TMP).

I should probably have addressed this in the patch to GetSpecialSystemDirectory in bug 351614. nsLocalFile::InitWithNativePath would be another possibility, but that is used a _lot_, especially on startup, and an extra check there might reduce performance (even if only a little).
Severity: major → minor
Status: UNCONFIRMED → NEW
Component: Download Manager → XPCOM
Ever confirmed: true
Product: Firefox → Core
Version: unspecified → Trunk
QA Contact: download.manager → xpcom
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Attachment #245899 - Flags: review?(mozilla)
Interesting. Isn't there some way to init a local file with forward slashes?
That patch was actually a quick demo live from Warpstock Europe, so I didn't do too much debugging before. :-)

But no, there is the !FindCharInReadable('/', begin, end) test in nsLocalFile::InitWithNativePath that causes file to be NULL if a forward slash occurs. I don't know what that test is there but I don't know what might happen when removing it. I guess we could also leave that test in and add another test and workaround forward slashes later in InitWithNativePath but that gets called a lot and as the problem only happens for TMP I prefered the solution in SpecialSystemDirectory.cpp.
Comment on attachment 245899 [details] [diff] [review]
use XPCOM string classes for forward slash replacment

r/sr=mkaply (OS/2 only)
Attachment #245899 - Flags: review?(mozilla) → review+
Fixed on trunk
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment on attachment 245899 [details] [diff] [review]
use XPCOM string classes for forward slash replacment

I hope I am not too late, but as this is OS/2 only and worked on trunk for 10 days, it would be great to get this into branch, too.
Attachment #245899 - Flags: approval1.8.1.1?
Comment on attachment 245899 [details] [diff] [review]
use XPCOM string classes for forward slash replacment

Approved for 1.8 branch, a=jay for drivers.  Please land ASAP as code freeze is  today. Thanks!
Attachment #245899 - Flags: approval1.8.1.1? → approval1.8.1.1+
OK, thanks. Fixed on branch, too. (Still on time it seems.)
Keywords: fixed1.8.1.1
Verified fixed using Firefox 2.0.0.1 and current "Gecko/20061222 Minefield/3.0a2pre" on OS/2.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.