166529 - symlinks in FTP directory don't have time

Status: VERIFIED FIXED

(Core Graveyard :: Networking: FTP, defect)

Description

[Boris 'pi' Piwinger]

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.1b) Gecko/2002090118

I just looked at ftp://ftp.complex.is/pub/ and found various entries without time.

I compared ncftp and found that those entries are symlinks (which you cannot see
in Mozilla). So either there should be the time (when the linked files changed)
or a display to which files those links refer to.

pi

Comment 1

[Bradley Baetz (:bbaetz)]

was this there beforehand? Ie did the new parser lose it?

Comment 2

[Boris 'pi' Piwinger]

I don't think I have seen this before, but I am not 100%.

pi

Comment 3

[Doug Turner (:dougt)]

Attachment: Proposed patch v.1

Fixes problem with LS symlinks.
It also fixes a problem with any filename with a space.

Comment 4

[Darin Fisher]

Comment on attachment 97760 [details] [diff] [review]
Proposed patch v.1

>Index: nsFTPDirListingConv.cpp

>+        char* offset = strstr(result.fe_fname, " -> ");
>+        if (offset) {
>+            result.fe_fnlen = offset - result.fe_fname;
>+        }

looks like offset can be declared |const char *|, which
should really be preferred over |char *| since you aren't
going to modify any of the bytes.  also, this avoids the
potential bustage if some platform's |strstr| returned 
|const char *| instead of |char *|. 


>+        aString.Append('\"');
>+        aString.Append(Substring(result.fe_fname, result.fe_fname+result.fe_fnlen));
>+        aString.Append('\"');
>+
>         aString.Append(' ');

this is just begging for a dependent concatenation!

Comment 5

[Doug Turner (:dougt)]

Attachment: addresses darin's comments

Comment 6

[Darin Fisher]

Comment on attachment 97786 [details] [diff] [review]
addresses darin's comments

sr=darin

Comment 7

[Darin Fisher]

Comment on attachment 97786 [details] [diff] [review]
addresses darin's comments

>Index: nsFTPDirListingConv.cpp

>+        aString.Append(nsDependentCString("\"") +
>+                       Substring(result.fe_fname, result.fe_fname+result.fe_fnlen) +
>+                       nsDependentCString("\" "));

whoops... i sr'd too fast... use NS_LITERAL_CSTRING instead of
nsDependentCString.
reason: NS_LITERAL_CSTRING uses sizeof to determine string length instead of
calling
strlen.

Comment 8

[Doug Turner (:dougt)]

Attachment: v.3

Comment 9

[Darin Fisher]

Comment on attachment 97802 [details] [diff] [review]
v.3

sr=darin

Comment 10

[neeti]

Comment on attachment 97802 [details] [diff] [review]
v.3

r=neeti

Comment 11

[Asa Dotzler [:asa]]

we need this for 1.2alpha

Comment 12

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 97802 [details] [diff] [review]
v.3

Taking Asa's comment as implying approval (seems good to me too). 
a=[e-mail]

Comment 13

[Doug Turner (:dougt)]

Checking in nsFTPDirListingConv.cpp;
/cvsroot/mozilla/netwerk/streamconv/converters/nsFTPDirListingConv.cpp,v  <-- 
nsFTPDirListingConv.cpp
new revision: 1.75; previous revision: 1.74
done


Thanks

Comment 14

[Boris 'pi' Piwinger]

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.1b) Gecko/2002090504

Bug was also in Windows, adjusting Platform/OS.

Fix works in version above. Don't have Linux RPMs to check yet.

Maybe one remarke (which might be a dupe or a new bug): The folder symbol in
front of the symlinks is confusing.

That was the fasted fix to a bug I have seen, great job!

pi

Comment 15

[benc]

(wonders if anyone has a good icon for links...)

Comment 16

[Boris 'pi' Piwinger]

I don't have icons, but I think some kind of arrow would do. Or anything else
unique. WS_FTP has an icon which looks like this: left half is folder icon,
right half looks like binary file.

pi

Comment 17

[Boris 'pi' Piwinger]

Verifying as reporter:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.2a) Gecko/2002091015
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.2a) Gecko/2002091014

Do we have a bug about the symlinks icons or should I file a new one?

pi

Comment 18

[benc]

Bug 133835.