Closed Bug 162025 Opened 23 years ago Closed 22 years ago

UNC Based Profiles don't work

Categories

(Core :: XPCOM, defect, P2)

x86
Windows 2000
defect

Tracking

()

VERIFIED FIXED
mozilla1.4final

People

(Reporter: mdhorton, Assigned: darin.moz)

References

Details

(Keywords: helpwanted, relnote)

Attachments

(1 file, 4 obsolete files)

I am using win2K pro on a linux samba domain. Every time I reboot my computer and start Mozilla I get the Mozilla Profile Manager and it asks me to select a profile. There is always a default profile which is the profile I was using before the reboot but when I select it and click start Mozilla I get an error saying "Mozilla cannot use the profile "default" because the directory containing the profile cannot be found. Please choose another profile or create a new one." So I create a new one and reboot and I get the same thing except this time it now has two profiles listed and both of them give me this error, so I create a new one and so on in a loop. I have deleted all profiles and started new and it still doesn't remember where my profile is from the last windows session. I am getting very frustrated with this and I cannot book mark any links as they will not be there after a reboot. Please Help.
Reporter: could you please read the Bug Writing Guidelines at http://www.mozilla.org/quality/bug-writing-guidelines.html to see the kinds of information we need in a bug report. Please report back with more information (like BuildID and steps to reproduce) after reading those guidelines and consider using the Bugzilla Helper to report future bugs. The Helper can be found at http://www.mozilla.org/quality/help/bug-form.html Thanks for your help in testing Mozilla. Also, are you using quicklaunch feature?
I am using build 2002072104. I have been having this problem since version 1. I have not tried to reproduce this on other machines as I only have a small network. I am using quick launch. This computer is a Linux Samba domain member and as such the user profile is stored on the server, which seems to be a problem for Mozilla. After a reboot it doesn't know where the profile is. I have another computer which is not a domain member and it works just fine. This seems related to the fact this computers profile is copied back to the server each time it is rebooted.
Please try disabling quicklaunch. As mentioned in Release Notes (http://mozilla.org/releases/mozilla1.1b/): > * Use of Mozilla's "quicklaunch" (AKA "turbo") mode may cause the deletion > of user preferences. It is recommended that you do not run quicklaunch > until this bug is fixed. (Bug 155080) This may be related to your problem. Also, please read bug 97045 and see if it describes your problem or not.
I am not using Quick Launch. I read some of these other bug reports and the profile I am creating is not being deleted. Mozilla just forgets where it is. If I create a new profile after a reboot and then copy the contents of the original profile I get all my settings back. But on the next reboot it loses it again. I have searched through the win2K registry and I cannot find where Mozilla is keeping this info. Can you tell me where Mozilla saves the info in the registry? Or does it save this info some where else?
Mozilla keeps its data in %USERDATA%\Application Data\Mozilla, %USERDATA% usually being c:\documents and settings\%USERNAME%. Could verify if that durectory withits subtree is properly copied back and forth when you reboot?
IT is keeping the profile there, and it is being copied, however on a reboot Mozilla says the profile isn't there. Even though I can find it on the drive, Mozilla says the directory containing the profile cannot be found. It's like Mozilla forgets where it stored the profile between boots (actually I don't even have to reboot, all I did do is logoff).
That is really weird, and I cannot find anything like that back in the database. I'm putting a helpwanted keyword on this, and hope somebody with Mozilla set up in a way similear to yours will confirm it. severity --> major.
Severity: normal → major
Keywords: helpwanted
I would like to ask some questions similar to one already posted, it might help me to give you some further info. Mozilla knows where the profile is stored until I reboot. So, where does Mozilla keep this info? Is it in the system registry? If so, I haven't found it. I have only found references to where the executable is and nothing else. Also, if I delete the profile, and then restart Mozilla, it says it found a Netscape profile, would I like to convert it. If it knows where the Netscape profile is, why doesn't it know where it's own profile is? Oh well any help on this will be appreciated. I really am trying to get away from IE and I have come up with a temporary workaround as already posted, so I guess I can live with this for now. By the way this didn't seem to start until I upgraded to version 1, if that's any help.
Mozilla doesn't store much information in Windows registry; it uses it's own "registry.dat" located in the %AppData%\Mozilla directory. Hmm, maybe that's the problem. 1) Log in with your network username 2) Open Regedit and go to HKCU\Software\Microsoft\Windows\Shell Folders 3) Write down the value of "AppData" string value 4) Go to the directory mentioned in AppData 5) Verify that directory named "Mozilla" exists there and it contains "registry.dat" file.
All I have there is HKEY_CURRENT_USER\Software\Microsoft\Windows\CurrentVersion, and HKEY_CURRENT_USER\Software\Microsoft\Windows\ShellNoRoam. I don't have a key by the name HKCU\Software\Microsoft\Windows\Shell Folders. Under the HKEY_CURRENT_USER\Software\Microsoft\Windows\CurrentVersion key I have a key called SHELL EXTENSIONS but no value APPDATA. This is probably why Mozilla doesn't remember after a reboot, it can't find the path cause none is being saved. I had tried to uninstall Mozilla and re-install it earlier today, but it made no difference. If you can tell me the exact string wording, I can create this key and see if that fixes it.
One more thing. There is a "registry.dat" file in C:\Documents and Settings\%username%\Application Data\Mozilla. It was last written to 08/11/02 7:40PM. So it is being used by Mozilla.
It sounds like you're an expierenced person and can do some advanced checks. I would suggest going to www.sysinternals.com and downloading the program called "file monitor". Then, launch iy, launch Mozilla, save Filemon's log to a file right after Mozilla claims it can't find a profile, and attach it to this bug. Then, we'll see exactly where Mozilla tries to look for a profile and that might give us a clue why it doesn't find it...
Sorry it has taken me so long to get back to you. I forgot to save the output from filemon, but I was able to figure out what is happening. This may be only related to my machine, however I bet it could be duplicated. The problem seems to be a missunderstanding of how win2k handles profiles. Win2k setup allows you to join a domain during the install process, which does save some time. However, when this profile is setup it's path is C:\Documents and Settings\username.domainname. Win2k never creates a profile for C:\Documents and Settings\username. So, you would think it would be a simple matter to just create the user "someuser", but when you create this user win2k makes it C:\Documents and Settings\username.machinename. Here lies the problem. Mozilla is looking for the profile in C:\Documents and Settings\username, which does not exist. I tried to change the profile path in the registry.dat file located in C:\Documents and Settings\username\Mozilla, but it is changed back everytime you restart Mozilla. I have, I think, fixed my problem by creating a directory C:\Documents and Settings\username. This seems to keep the profile in tact between reboots, but I am doing more testing. This fix might also fix some of the other bugs with similar problems. I think this is some problem with the way Mozilla gets it's info on where the profile is during the install. I wish I knew more about the MS win2k profile thing to be of more help to you. If there is anymore testing you would like for me to do, let me know. For now I am fixed, thank god, it was really getting to be a hassle to have to start all over with Mozilla after each reboot. I really like this software, IT'S GREAT. Thanks. Mike
Mike, we really appreciate your efforts on figuring the case of the problem. It will be so much easier for the development team to fix this bug now when you performed such an extensive investigation. Good job, and thank you for your help on making Mozilla the best browser ever! ==> Changing Component to Profile Manager BackEnd
Status: UNCONFIRMED → NEW
Component: Profile Manager FrontEnd → Profile Manager BackEnd
Ever confirmed: true
Whiteboard: Need people with w2K on a Samba domain to reproduce the bug
reassign to component owner
Assignee: ben → ccarlen
*** Bug 100395 has been marked as a duplicate of this bug. ***
*** Bug 178267 has been marked as a duplicate of this bug. ***
anyone want to dupe bug 101953 and bug 110832 as well? i think they all talk about the same thing. also, any chance in nominating for 1.2? this prevents mozilla from entering the enterprise space.
Depends on: 101953
*** Bug 110832 has been marked as a duplicate of this bug. ***
Re comment 18: bug 101953 is a meta bug and therefore not a duplicate, rather this bug depends on bug 101953. I also doubt that this will be fixed in 1.2, no patches are available.
*** Bug 178416 has been marked as a duplicate of this bug. ***
Howdy, We've also been seeing the same problem as Mike intermittently for some users on our local Win2k domain. I've tried to verify that the fault is caused by Mozilla assuming that the name of the name of the local profile store (ie as defined in %USERDATA%) is the same as the logged-in user's username (ie as defined in %USERNAME%). To force the local profile store to have a different name, two new folders were created as domain administrator of the form <username> and <username>.<domain> and all access rights to those directories were removed. Then the test user logged in. Unfortunately, whilst Windows chose a different name for the profile store Mozilla was still able to create a new profile as normal; moreover once closed Mozilla was able to locate the profile that it had previously stored. Looking at the code in xpcom/io/nsSpecialSystemDirectory.cpp, Mozilla seems to be doing the right thing by asking the relevant MS API (see <a href="http://msdn.microsoft.com/library/default.asp?url=/library/en-us/shellcc/platform/Shell/reference/enums/csidl.asp">here</a>) for the location of the user's profile directory rather than using a heuristic to determine it for itself. So, I don't know what's causing this problem and would like to get it fixed. If anyone can suggest some pertinent tests I can try them out locally. We're using the stock 1.0 Mozilla win32 release and aren't using QuickLaunch. We're using a set of Win2k domain controllers with Samba+Linux boxes providing homedirectory access (mapped as H:\).
As Torben suggest on bug 110832 I am attaching my patch from that bug here. Note that this patch is about a year old and is incomplete so is probably most useful only at a starting point. ---- This is just a partial fix. Komodo (an app built on Mozilla-the-framework), for now, took the route of working around this problem by falling back to CSIDL_LOCAL_APPDATA on Windows if CSIDL_APPDATA was a UNC path (the presumption being that CSIDL_LOCAL_APPDATA directory is *not* a UNC path <fingers-crossed>). Here is that Mozilla patch that Komodo is using for that: http://bugs.activestate.com/Komodo/showattachment.cgi?attach_id=163 and the related Komodo bug: http://bugs.activestate.com/Komodo/show_bug.cgi?id=7547
another possible dupe, bug 79419 . . .
*** Bug 79419 has been marked as a duplicate of this bug. ***
*** Bug 180926 has been marked as a duplicate of this bug. ***
*** Bug 181086 has been marked as a duplicate of this bug. ***
This bug is blocked by bug 101953 and I'm not sure that's appropriate. I think we are looking at two different unrelated problems here and I'll hopefully prove my theory. First, let me digress. I manage a windows domain environment of 2000 machines with 8000 roaming users (a college with loads of students). Roaming profiles are used. We rolled out Mozilla in the fall and have had horrible problems based on the mozilla profile issue (and the lack of spell checker, but that's a different bug :). Problem #1 (which is bug 101953 and is blocking this bug): Mozilla can't handle UNC pathnames anywhere apparently. If a system administrator assigns a GPO (group policy object) redirecting the application data (%appdata%) to a UNC, Mozilla won't work. It can't find the profile directory. (related comments on this are in bug 110832, which was marked a dupe of this bug recently). Problem #2: Profile locations are stored in registry.dat as absolute paths, like c:\windows\profiles\user\application data\mozilla\... etc. As stated in comment #22, Mozilla at first does the right thing and locates registry.dat by finding appdata by basically expanding %appdata%. The problem is, after that, the paths within that registry.dat file are used. This is a problem because the path that %appdata% points to can change in Windows NT/2000/XP. For instance, if a user does not logout and the machine is reset, the profile server is rebooted during logout, power failure, some corruption is evident, the moon is full, or a 12-sided die lands on a certain number, then on the next logon, Windows will copy the roaming profile to a new profile directory on %systemdrive%. In our environment, we get a sequence of user, user.domain, user.domain.000, user.domain.001, etc... So the profile with a registry.dat file pointing to a different place is copied into the current %appdata% during logon, and mozilla gets confused and throws up the profile manager when started. If you click the one profile listed, it can't find it, because %appdata% has changed since registry.dat was initially created in the user's profile and hence moz can't find the profile data. So, because of this, I believe these two bugs are unrelated and can be squashed independently. At least that's my theory! :)
Flags: blocking1.3a?
Flags: blocking1.3a? → blocking1.3a-
correcting relationship bug 101953 is a meta bug, and as I understand it, exists because there are two generations of file i/o libraries, one of which does not do UNC's correctly, the other of which does. File URL's work fine w/ UNC's, so I know that this does work for some areas in mozilla. Based on these problem descriptions, I think we can set this up and reproduce it internally, given some time, if that is what is needed next to fix this bug.
Blocks: 101953
No longer depends on: 101953
No longer blocks: 101953
Depends on: 101953
QA Contact: ktrina → gbush
I had this one bite me in the *ss yesterday, though with a slightly different effect. I took me a day to trace it back to this bug and figure out a workaround. Here's my situation: Fresh install of WinXP Pro SP1. I installed the latest moz nightly (20030129, I think). Fire it up and it wants me to create a profile. Clicking "Finish" to create the profile does nothing. Looking closer, I realized that I couldn't see anything displayed where I expected the "your profile will be created here" message to be. Clicking "Choose Folder" didn't do anything either (no dialog). At this point, I start to panic, thinking that I'll be forced to use IE from now on. Here's how I worked around it: 1) Find out where %APPDATA% goes (see comment 9) 2) Create "Mozilla\Profiles" folder under %APPDATA% 3) Create "Mozilla\Profiles" folder under C:\Documents and Settings\<user>\Application Data 4) Install Mozilla 1.3a 5) Start Profile Manager -> Create ... Choose Folder and select C:\Documents and Settings\<user>\Application Data\Mozilla\Profiles Finish. The install of 1.3a was because "Choose Folder" didn't work in the nightly I had, but did in 1.3a. I realize this probably isn't the place, but I have a couple of other observations coming out of this: 1) If %APPDATA%\Mozilla\Profiles doesn't exist, the Create Profile dialog doesn't display the message telling you where the new profile will be stored. 2) Moz should spit out some error messages when something goes wrong. A text label didn't display, two buttons did nothing, and the UI acted as if everything was normal. I even tried launching with the -console option to see if anything showed up on STDERR (nothing did). Without some determination and luck, I would have never figured out what was going wrong.
i can't believe we have a patch for the unc problem burried over here...
Assignee: ccarlen → trentm
Component: Profile Manager BackEnd → XPCOM
Summary: Profile problem in Win2K → UNC Based Profiles don't work
Comment on attachment 105344 [details] [diff] [review] proposed partial fix to allow UNC paths dougt/darin: please look at this. the only change i'd make is: >+ if (*(slash+1) == '\\') { >+ slash += 2; >+ slash = _mbschr(slash, '\\'); >+ slash += 1; >+ } else { >+ slash += 1; >+ } > slash = _mbschr(slash, '\\'); to: if (*(slash+1) == '\\') { slash += 2; slash = _mbschr(slash, '\\'); } slash = _mbschr(slash + 1, '\\');
Attachment #105344 - Flags: superreview?(darin)
Attachment #105344 - Flags: review?(dougt)
Comment on attachment 105344 [details] [diff] [review] proposed partial fix to allow UNC paths >Index: nsLocalFileWin.cpp > unsigned char* slash = _mbschr((const unsigned char*) mResolvedPath.get(), '\\'); >+ if (*(slash+1) == '\\') { >+ slash += 2; >+ slash = _mbschr(slash, '\\'); >+ slash += 1; >+ } else { >+ slash += 1; >+ } couple comments: 1- your patch doesn't apply cleanly. (conflicts with version 1.70 from Jan 2002!) 2- make sure you null check the return value of _mbschr (which the current code does) 3- increment |slash| in only one place... i.e.: unsigned char* slash = _mbschr((const unsigned char*) mResolvedPath.get(), '\\'); // XXX null check return value! if (slash[1] == '\\') { slash = _mbschr(slash + 2, '\\'); // XXX null check return value! } slash++;
Attachment #105344 - Flags: superreview?(darin)
Attachment #105344 - Flags: superreview-
Attachment #105344 - Flags: review?(dougt)
is this fixable for 1.3?
Flags: blocking1.3?
Keywords: mozilla1.2
Flags: blocking1.3? → blocking1.3-
I just saw that asa denied to fix the UNC problem for 1.3. I wonder if anybody at moz.org is aware that this very bug prevents every place with decent (meaning centralized profiles without local copy) from using mozilla at all ? Mozilla just does not start. This is realy sad and is denying mozilla a whole segement of the market. What is ironic, is that it is probably exactly the places which would be most friendly to mozilla as they have people who are realy into big networks, otherwhise they would not pick that particular methode of constructing a windows setup. A further problem with this bug is that mozilla just dies from it without generating any errors what so ever, so most people will not figure that it is this very bug which is responsible and will thus just conclude that moz is broken.
This bug has now been unsolved for a whooping 6 months or so... it is incredible that noone has been able to resolve this one by now. Just the fact that it was denied fixing for 1.3 shows that depending on volunteers for software simply does not work all the way. Well Mozilla is out of reach for my 800 potential users, they are then instead becoming happy MSIE and Opera users. Sad, because Mozilla is a pretty decent browser...
Blocks: 101953
No longer depends on: 101953
i spoke to trent about this bug. he's not planning to work on this anytime soon. -> suresh
Assignee: trentm → suresh
Priority: -- → P2
Target Milestone: --- → mozilla1.4alpha
Is this still qawanted?
I have to chime in to agree with comments 36 and 37. This bug heavily discourages Mozilla use in Windows domain environments with redirected user profiles... which is probably most Windows shops. It *is* possible to work around it, if you have access to a cooperative domain administrator and are willing to add logon scripts and change domain policies just to support Mozilla. However, the nature of the workaround needed is not at all obvious. The Windows profile redirection to a UNC path is invisible to the user, Mozilla's failure mode gives NO ERROR MESSAGE AT ALL, and the release notes (IIRC) don't say anything about these symptoms. This all makes the problem awfully hard to diagnose and fix. If y'all don't document the problem and symptoms in the release notes, and point the user to some documentation for a workaround, Mozilla's penetration into the Windows domain market is gonna be very slow indeed. At the very least, Mozilla needs to throw an informative error message when it chokes on a UNC profile path. Don't let this slip beyond 1.4!
To cut to the important things (i.e. money), what does Netscape 7.02 do in this regard? I'm going to assume it doesn't understand UNC pathnames either, so one must ask what AOL-TimeWarner want to do with Netscape in the corporate Windows environment.
This should have a release note for 1.3 final. Could someone please submit proposed text for one? TIA.
Keywords: relnote
Suggested release not text: Known Problems: Windows: UNC paths are not fully supported. In a domain environment with redirected Windows user profiles, this can lead to Mozilla failing to create or recognize a Mozilla profile, causing Mozilla to end immediately after launch. If this occurs, change the domain policy to no longer redirect the "Application Data" folder.
Re the suggested release text, also note comment #28, part 2, that just unredirecting (nice word) the appdata folder is not enough. Problems will still occur because windows often changes the path to the windows profile itself on the local machine. A workaround for this I later found was to ensure the mozilla profile is stored in a fixed location, like user home dir that has been assigned a drive letter. That way everything but registry.dat is moved out of the roaming profile. This can be automated via a cmd likemozilla -CreateProfile "default z:\mozilla"Note the double quotes are critical. Getting the mozilla profile out of the roaming profile is also important for logon/logout times since all the changed **** in roaming profiles is copied down/up during logon/logout and if user has large cache dir, it's a real time killer.After our dropping appdata back to unredirected on our domain's GPO and moving all user moz profiles to z:\mozilla, our mozilla problems have finally stopped. But let me tell you it's been a lot of work and I took a big reputation beating for deciding to deploy mozilla in my enterprise of 2000 desktops. I am a big moz fan and I now regret my initial decision. I can't imagine more than a handful of enterprise domain admins, if that, sticking it out and putting the work in to make mozilla work in an enterprise. btw, Asa must be busy as hell, and I'm sure can't have time to read all comments. The status whiteboard of this bug is very misleading, as well as first few comments. It implies this bug only affects w2k users on samba domains, which is probably a very small set of situations. Would changing the whiteboard help?
Perhaps it would help if we had a new clean building version of the patch?
Thank you for the informative post. If you don't like the proposed relnote, how about this one? Relnote: Support for profiles referenced by UNC paths is not implemented. For workarounds, see bug 162025.
Whiteboard: Need people with w2K on a Samba domain to reproduce the bug
Flags: blocking1.4a?
Keywords: mozilla1.3
Attached patch updated patch (obsolete) — Splinter Review
the important part is: @@ -642,11 +630,11 @@
Attachment #105344 - Attachment is obsolete: true
Attachment #116142 - Flags: superreview?(darin)
Attachment #116142 - Flags: review?(suresh)
Attachment #116142 - Flags: review?(suresh) → review?(dougt)
Comment on attachment 116142 [details] [diff] [review] updated patch i don't like the idea of redefining strrchr. how about just create a new method my_strrchhr and map that to _mbstrchr on windows or XP and for the other platforms just call strrchr. (what I really don't understand is why can we call strrchr on everything but windows and os2). I would argue against landing the nsFileSpec changes. It is dead. continuely fixing it does not give anyone any reason to migrate. Do you have any test cases I can run? Please don't land this until after i land my minimo branch. I will be moving nsFileSpec into a different directory.
I can answer you strrchr question. As you know, on OS/2 and Windows, backslash is the path delimeter. Unfortunately, backslash is not in the control character section, it is at position 5C. Because of this it can be the second byte of a double byte character. C:\name\naA\me where \/ These represent one character and if you used strrchr, you would get the wrong backslash. There is an easy workaround to this on OS/2 and windows (WinPrevChar and PrevChar) which is why we had to implement our own mbsrchr in nsLocalFileOS2 (and Win). I haven't read this bug completely to see what the issues are, but I ran across doug's comment and thought I would mention this.
->timeless, as he has the patch :)
Assignee: suresh → timeless
dougt, is mkaply's response sufficient?
Flags: blocking1.4a? → blocking1.4a-
Comment on attachment 116142 [details] [diff] [review] updated patch >+#if defined(XP_WIN) || defined(XP_OS2) >+#define strrchr(xstring, xcharacter) (char*) _mbsrchr((const unsigned char *) xstring, xcharacter) >+#endif Perhaps it would be better to define a new macro that's called neither |strrchr| nor |_mbsrchr|? I could imagine this causing problems if strrchr is already a macro, or in case somebody really wants strrchr elsewhere in the file at some point in the future. >Index: mozilla/xpcom/io/nsFileSpecWin.cpp > src = (char*)ioPath; > >+ // Convert '/' to '\'. (Must check EVERY character: consider UNC path >+ // case.) >+ while (*src) >+ { >+ if (*src == '/') >+ *src = '\\'; >+ ++src; May I suggest: for (src = (char*)ioPath; *src; ++src) if (*src == '/') *src = '\\'; ? How have you tested UNC-based profiles to see that they work with this patch? Also, could you attach a new patch against the trunk (since files have moved)? It would be good to get this in for 1.4beta.
forget macro'ing strrchr... all usages are to locate the last path segment. just add a function like this: const char *GetLastPathSegment(const char *path, const char separator) { #if defined(XP_PC) return _mbsrchr(path, separator); #else return strrchr(path, separator); #endif } why in the world is |separator| a parameter... oh well, this is dead code anyways :-/
Flags: blocking1.4?
adt: nsbeta1-
Keywords: nsbeta1nsbeta1-
Folks, It seems to me, that the cause of the bug is well researched, documented and there is even a patch, yet fixing gets pushed out further and further ... this is realy annoying to me ... we have hundreds of people here at our site who would love to use mozilla and keep requesting it's deployment, yet we can't because of moz's inability to handle UNC paths. I assume manywell managed Windows setups at large sites keep their user profiles on the server behind a UNC path witout copiing them to the local drive on every logon ... Please, Please .... Make 1.4 work for us
-> me (i'll see what i can do about cleaning up the patch and getting it in for 1.4 beta or at least final) timeless: feel free to jump back in if you have free cycles.
Assignee: timeless → darin
Target Milestone: mozilla1.4alpha → mozilla1.4beta
Flags: blocking1.4b-
Flags: blocking1.4b+
Flags: blocking1.4?
Flags: blocking1.4+
*** Bug 156018 has been marked as a duplicate of this bug. ***
Status: NEW → ASSIGNED
Target Milestone: mozilla1.4beta → mozilla1.4final
Attached patch v2 patch (obsolete) — Splinter Review
with this patch i'm able to create and use a profile stored under a UNC file path. i haven't fully tested it with all features of the browser, but it seems to work for most things. the biggest change from the previous patch is corrections to the nsLocalFile::Create method. when attempting to create parent directories, we have to start at the right spot to avoid trying to create '\', '\\', '\\host', or '\\host\volume' since CreateDirectory will fail on all those inputs. also, Create should tolerate access denied errors since windows will return access denied for parent directories that the calling process does not have permission to modify.
Attachment #116142 - Attachment is obsolete: true
Attachment #116142 - Flags: superreview?(darin)
Attachment #116142 - Flags: review?(dougt)
Attached patch v2.1 patch (obsolete) — Splinter Review
incorporating dbaron's suggested for loop.
Attachment #123115 - Attachment is obsolete: true
Attachment #123117 - Flags: review?(dougt)
Comment on attachment 123117 [details] [diff] [review] v2.1 patch nit: drop "nested" + // create nested directories to target nit: is the start here pointing at the wrong thing in both of the diagrams? + // - UNC path: \\machine\volume\some\path\on\this\drive + // ^--- start here what if the machine part of the file path is empty? for example: "\\\" +// return pointer to last instance of the +static inline char *GetLastSeparator(const char *str, char sep) + nit: it is probably faster looking at the second char since most paths will have a | at 1. #if defined(XP_WIN) || defined(XP_OS2) // If it's a drive letter use the colon notation. - if (!leafPointer && result[2] == 0 && result[1] == '|') + if (!leafPointer && result[1] == '|' && result[2] == 0) result[1] = ':'; #endif
Is it safe to look at result[2] first, or could that UMR? /be
yeah, i assumed that was the reason why that change was made. i just carried it over from the original patch. dougt: nice catch on the '\\\' cases... i'll make sure to test out those, and verify that they work correctly. start here is correct. you want to start creating \\machine\volume\some and then \\machine\volume\some\path and so on. if you try to create the volume, CreateDirectory will fail.
Attached patch v2.2 patchSplinter Review
revised per comments from dougt. my system requires a machine name, \\\volume is not a valid UNC path according to my win2k machine. that aside, i think the code would handle \\\volume just fine.
Attachment #123117 - Attachment is obsolete: true
Attachment #123117 - Flags: review?(dougt)
Attachment #123242 - Flags: review?(dougt)
Comment on attachment 123242 [details] [diff] [review] v2.2 patch Looks good.
Attachment #123242 - Flags: review?(dougt) → review+
Attachment #123242 - Flags: superreview?(dbaron)
Comment on attachment 123242 [details] [diff] [review] v2.2 patch >Index: io/nsLocalFileWin.cpp >+ path++; I'd prefer |++path;| both for consistency with surrounding code and because of bug 78032 comment 1. >Index: obsolete/nsFileSpec.cpp >+// return pointer to last instance of the given separator >+static inline char *GetLastSeparator(const char *str, char sep) |static inline| seems redundant -- how about just |inline|? >+{ >+#if defined(XP_WIN) || defined(XP_OS2) >+ return (char*) _mbsrchr((const unsigned char *) str, sep); >+#else >+ return (char*) strrchr(str, sep); No need for the cast here, right?
Attachment #123242 - Flags: superreview?(dbaron) → superreview+
dbaron: >>Index: io/nsLocalFileWin.cpp >>+ path++; > >I'd prefer |++path;| both for consistency with surrounding code and because of >bug 78032 comment 1. for consistency ok, but ++path would not optimize any better. this is straight pointer arithmetic here.. C++ operator overloading is not involved.. nor is any assignment statement. >>Index: obsolete/nsFileSpec.cpp >>+// return pointer to last instance of the given separator >>+static inline char *GetLastSeparator(const char *str, char sep) > >|static inline| seems redundant -- how about just |inline|? compiler can choose to ignore |inline|, in which case i would be afraid of the symbol having extern linkage. |static| seems prudent to avoid possible namespace collisions on really lame build environments... ok, i made that up... but we use |static inline| elsewhere, so for consistency ;-) >>+{ >>+#if defined(XP_WIN) || defined(XP_OS2) >>+ return (char*) _mbsrchr((const unsigned char *) str, sep); >>+#else >>+ return (char*) strrchr(str, sep); > >No need for the cast here, right? _mbsrchr returns |const unsigned char*|, so we need the cast for that, and on some platforms |strrchr| returns |const char*| so we need the cast there as well. "man strrchr" on worms (SunOS 5.8) says that ISO C++ declares strrchr to return |const char*|, and i think i've been burnt by this before on Solaris. so, i'd like to just make the postfix to prefix change if that is ok with you.
Comment on attachment 123242 [details] [diff] [review] v2.2 patch requesting drivers approval for 1.4 final. thx!
Attachment #123242 - Flags: approval1.4?
Comment on attachment 123242 [details] [diff] [review] v2.2 patch a=asa (on behalf of drivers) for checkin to 1.4
Attachment #123242 - Flags: approval1.4? → approval1.4+
ok, patch is in. marking FIXED. i can't guarantee that everything will work correctly now, but as far as i can tell everything does. please help me test this patch in tomorrows nightly build. if you find a problem please file a new bug against me. thanks!
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
I realize this is a case of "too little- too late" (and possibly irrelevant) but is there a reason why you cannot use the path functions in the "Shell Lightweight API" (Shlwapi.dll) library? It seems to have all the path handling/validation/manipulating functions that we all have to do over and over. Also, Mozilla.exe effectively depends on it via a dependency on shell32.dll
I had submitted bug 207282 which appears to be a duplicate of this bug. I have since installed Mozilla 1.4 RC1 and can confirm that this problem has been resolved. I hope that somebody with the power can convince Netscape the importance of implementing this patch as quickly as possible. Netscape 7 is the first Netscape browser that can readily compete with Internet Explorer. Unfortunately this bug, which is still present in Netscape 7.02, basically makes this unusable in a corporate environment with roaming profiles (which is the majority). I'm a web developer that has had to support Netscape 4.x versions because some of our clients have used that as their corporate standard. I have recently been told that a major client, Bristol-Meyers Squibb (BMS), will be changing to IE 6. It is unfortunate that Netscape 7 couldn't even be considered as an option due to this problem. I hope that somebody at Netscape really understands the serious nature of this problem. I think it should be considered as critical as any security bug that is found. Thanks for letting me vent, and for at least patching Mozilla. Now I can test my sites with the new version and hope that Netscape will soon follow. Michael
*** Bug 207282 has been marked as a duplicate of this bug. ***
This isn't really the place to talk about Netscape releases. If you want such information, go to a site like Mozillazine.org. (To answer your question, many think the next Netscape release will be based on Mozilla 1.4.)
verified per comment #70
Status: RESOLVED → VERIFIED
*** Bug 197308 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: