Closed Bug 209110 Opened 21 years ago Closed 20 years ago

GetNativeTarget() function under win32 returns extra '\\'

Categories

(Core :: XPCOM, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: ssu0262, Assigned: jonitis)

Details

Attachments

(2 files, 2 obsolete files)

On Win32, when calling GetNativeTarget() on a .lnk to a folder (my test case was
"c:\\"), GetNativeTarget() will return the native path with an extra backslash:
  "c:\\\\"

When calling on a non root folder, such as "C:\\Program Files", it returns:
  "C:\\Program Files\\"
sean, did you have a patch that fixes this problem?
not yet.  haven't had a chance to investigate it further.  had to deal with
other bugs first.

I'll try to get a patch going, unless you get to it before I do.
Attached patch diff -u (obsolete) — Splinter Review
Attached patch diff -uw (obsolete) — Splinter Review
The problem was that GetPath () was returning appended slash for the root
directory, but not for other directories. 
1. Added check for root directory. 
2. Removed tab symbols.
Attachment #127667 - Flags: superreview?(dougt)
Attachment #127667 - Flags: review?(ssu)
Comment on attachment 127667 [details] [diff] [review]
diff -u

why not just check for a trailing backslash regardless of the path?  If there's
already backslash, then skip the appending of a backslash.
Sure. That should be even safer.

BTW the code that checks for shortcuts (".lnk") is not safe because it uses case
sensitive comparison. 
Attached patch diff -uSplinter Review
Append trailing slash only if it is not already present
Attachment #127667 - Attachment is obsolete: true
Attachment #127668 - Attachment is obsolete: true
Attachment #127821 - Flags: superreview?(dougt)
Attachment #127821 - Flags: review?(ssu)
Comment on attachment 127821 [details] [diff] [review]
diff -u

+		     // For root directory slash is already appended

please fix comment.


+		     if (temp [strlen (temp) - 1] != '\\')

strlen() instead  - no space.

ssu, what bug did you run into with the double slash?
What is wrong with a commnet? It just describes the non standart behaviour of
root directories. The fact that we are not appending slash if one already exists
is obvious from the code.
dougt, I was testing the fix to bug 201381 and one of my tests revealed the
double backslash for a migrated bookmark to a shortcut to the root of a drive.
Comment on attachment 127821 [details] [diff] [review]
diff -u

I don't think you can test for the trailing backslash that way because if the
string is multibyte, it will not work.

strlen() should return the correct length, but then dereferencing using '[]' is
not multibyte safe because it's dereferencing bytes instead of characters.

Looks like there are many other parts of nsLocalFileWin.cpp that are not
multibyte safe... *sigh*

dougt, shouldn't we be using the string class here for all string manipulation?

r=ssu if dougt is okay with the change, since it's broken everywhere else
already.  not that I'm advocating checking in broken code.  Just that since
there's already brokeness, this shouodn't make it any worse.

If this is to be checked in, please comment that it is not mutibyte safe.
Attachment #127821 - Flags: review?(ssu) → review+
Attachment #127667 - Flags: superreview?(dougt)
Attachment #127667 - Flags: review?(ssu)
Comment on attachment 127821 [details] [diff] [review]
diff -u

fine.  check it in.
Attachment #127821 - Flags: superreview?(dougt) → superreview+
Can you check it in for me? I do not have access rights
On network printer path is the same:
user_pref("print.printer_\\TREE\\printer-queue.print_bgimages", false);
after next run is changed to:
user_pref("print.printer_\\\\TREE\\\\printer-queue.print_bgimages", false);
After few days my prefs.js had 23 megabytes of this junk!!!
I don't think that this is the right bug. I think you're suffering
from a regression based on converting prefs from js to properties.
I have had the same problem with our local network Filelist.. with IE it shows
"file:\\" + computer name + path... and with Mozilla "file:\\\\" and so on.. 
All other "\\" are correctly.. I see it ALLWAYS... no doubt there is a bug or
difference between IE and Mozilla... And I am using Win98 so it is a general
problem..
that too is an unrelated issue, further it isn't even a bug.
for people testing from home, create c:\test.lnk which points to c:\, then run
xpcshell:

js> a=Components.classes["@mozilla.org/file/local;1"].createInstance(Components.
interfaces.nsILocalFile)
[xpconnect wrapped nsILocalFile]
js> a.initWithPath("c:\\test.lnk")
js> a.path
c:\test.lnk
js> a.exists()
true
js> a.isSymlink()
true
js> a.path
c:\test.lnk
js> a.target
c:\test.lnk
js> a.followLinks
false
js> a.followLinks=true
true
js> a.target
C:\\

the last line, if this is working should only have one slash.
Assignee: dougt → Dainis_Jonitis
nice! you should attach that as a script that people can run to test this bug
(rather than typing it in manually from your transcript)
Attached file Testcase
Desktop>mozilla.17a\xpcshell.exe 209110.js || echo FAIL
built on Feb 19 2004 at 13:25:36
File resolved to a path containing two backslashes: C:\\
FAIL

Desktop>mozilla\xpcshell.exe 209110.js && echo PASS
built on Mar  2 2004 at 08:34:50
File resolved to a path with a single trailing backslash: C:\
PASS
mozilla/xpcom/io/nsLocalFileWin.cpp 	1.113
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: