115135 - file:// directory listings need ability to generate HTML directory listings.

Status: VERIFIED DUPLICATE of bug 102812

(Core :: Networking: File, defect, P2)

Description

[Judson Valeski]

Problem:
Currently, the only directory output format option for file:// URLs that resolve
to a directory, results in tree widget UI. 

In the embedded space, this means that one has to lug the appcomps lib around
just for dir listings (optimized windows build generates this lib to the tune of
200k bytes). If you don't have the appcomps lib, accessing a file:// url
resolving to a dir generates an Unknown Content Type Handler Dialog. The app
should be able to specify HTML as its dir output type (just like FTP).

This obviously has uses outside of the embedded space for those who want HTML
dir listings for file anyway, in their standalone app.

This should be toggleable via prefs.

Comment 1

[Doug Turner (:dougt)]

Attachment: proposed patch

Comment 2

[Doug Turner (:dougt)]

Attachment: proposed patch

Comment 3

[Doug Turner (:dougt)]

Attachment: patch for trunk.

Comment 4

[Doug Turner (:dougt)]

Attachment: patch to all.js - disables html directory views by default

Comment 5

[Darin Fisher]

Comment on attachment 61642 [details] [diff] [review]
patch for trunk.

nit alert!  otherwise, sr=darin


>Index: nsFileChannel.cpp

>+static NS_DEFINE_CID(kStreamConverterServiceCID,    NS_STREAMCONVERTERSERVICE_CID);

extra whitespace


>         PRBool directory;
> 		mFile->IsDirectory(&directory);
> 		if (directory) {
>-            mContentType = APPLICATION_HTTP_INDEX_FORMAT;
>+            if (mGenerateHTMLDirs)
>+                mContentType = "text/html";
>+            else
>+                mContentType = APPLICATION_HTTP_INDEX_FORMAT;

indentation problem


>     if (mRealListener) {
>+        if (mGenerateHTMLDirs)
>+        {
>+            PRBool directory;
>+		    mFile->IsDirectory(&directory);  // this stat should be cached and will not hit disk.
>+		    if (directory) {
>+                rv = SetStreamConverter();
>+                if (NS_FAILED(rv)) 

inconsistent braces and screwed indentation


>+    PRBool                              mGenerateHTMLDirs;

PRPackedBool ?


>Index: nsFileProtocolHandler.cpp

> static NS_DEFINE_CID(kStandardURLCID, NS_STANDARDURL_CID);
>-
>+static NS_DEFINE_CID(kPrefCID, NS_PREF_CID);
> ////////////////////////////////////////////////////////////////////////////////

why eliminate the space?

Comment 6

[Judson Valeski]

Comment on attachment 61643 [details] [diff] [review]
patch to all.js - disables html directory views by default

note that this will also make ftp dir listings be XUL on the trunk (currently
they're html).

Comment 7

[Darin Fisher]

bbaetz once mentioned that there are intl problems with the HTML viewer.  that
there are bugs that would make using the IndexedToHTML converter not work right
for file:// directory browsing.

i wish bbaetz were around to comment on this.  i suspect making this change will
generate a lot of regression bugs from non en-locale folks.

Comment 8

[Darin Fisher]

please don't enable XUL directory viewer... it is terribly broken right now.  it
makes current mozilla builds useless for browsing file:// URLs.  bbaetz is aware
of the problems, and plans to fix them... but, he's on vacation until february.
 so, unless someone else wants to own it, i think we should just disable it.

Comment 9

[Darin Fisher]

cc'ing bbaetz... maybe he'll happen to check his email before going off on
vacation ;)

Comment 10

[Bradley Baetz (:bbaetz)]

Err. Stop right here. I'm still here for another week :)

There are several bugs, and I'm in Toronto til Saturday evening. The relevent 
bugs (which I didn't get to fix because of exams) are:

bug 102812 - enable html directory listing for file urls. This is hard coded to 
be disabled, because it breaks on non-ascii directories. I think I have a fix 
for this, but didn't have time to test it (and since we only support non-ascii 
file listings on non english win2k installs, this means getting someone from the 
i18n team). If someone wants to comment out the stuff in nsDirectoryViewer.cpp 
which disables this for file, and play with changing the character encoding via 
the menu on a jp system, or otherwise debug this, that would be helpful (My fix, 
IIRC, changes it to add a META charset tag for the filesytem charset, or remove 
it, or something. I can't check from here)

Bug 110760 - directory listings shouldn't depend on libappcomps (which appears 
to be this bug). Looking _very_ quickly through the patch, this looks like what 
I was going to do, sort of. I was going to extend it to the other protocols 
though, and add a helper func. This would also fix bug 102826, and a few similar 
ones I have.

Please test this on a win2000-jp system first....

Also, please do not enable xul listings by default. They're broken. If someone 
gives me more hours in the day, I'll get to my rewrite in late Jan/early Feb, I 
hope.

From the patch itsself, you don't register as a pref observer. I was going to 
add a helper class (either to nsNetUtil.h, or a new header file) which could be 
shared amongst the protocols which would do this, and would return a boolean for 
whether html dir listings were currently enabled.

Comment 11

[Bradley Baetz (:bbaetz)]

Bug 78148 has some screenshots of what this looks like, btw, done by jbetak.

Comment 12

[Doug Turner (:dougt)]

The check against the preferred output should not only sit in the XUL content
viewer.  Suppose I do not want to ship xul and I want to produce html file://
content?  

The intl bug is orthoganal.  

Comment 13

[Bradley Baetz (:bbaetz)]

Right; the idea would be exactly as is in this patch, and that would hopefully 
fix places which don't go through teh xul context viewer (like send page and so 
on - see the other bugs).

i18n support isn't really orthogonal, since you cannot enable file:/// as html 
until those issues are fixed. The fix is unrelated, but its probably a 
precondition for getting this checked in.

Comment 14

[Doug Turner (:dougt)]

I don't know about a precondition of this checking, but maybe a precondition of
a mozilla milestone?

darin?

Comment 15

[Darin Fisher]

definitely, making this change would be a blocker for the next milestone.  we
need to fix the HTML viewer for intl users.  unfortunately, even a badly broken
XUL viewer is better than no directory viewer :(  still, i vote for making this
change.  i just wish we knew who was going to own getting the HTML viewer
fixed... cuz i don't think we want to wait until bbaetz gets back.

Comment 16

[Doug Turner (:dougt)]

chak is owning this now.  Thanks!

Comment 17

[Bradley Baetz (:bbaetz)]

OK, so I didn't get to look at this.

So: this patch will not remove the reliance on appcomps, becuase you'll still
need it for ftp dir listings. While this patch appears to remove the dependancy
for file:///, embedders are probably interested in ftp, too.

If the goal is to allow html file:///, just remove the hard coded stuff in
nsDirectoryViewer.cpp. If you want to do this generically, to remove the
dependancy, then you need to modify ftp/gopher as well.

Fixing the i18n issues (if any) is likely to be a couple of hours work for
someone who has access to a non en-US w2k version. If someone commits to doing
that by 0.9.8, then I have no problem with one of my two suggested fixes going in.

However, I do think that the patch as-is doesn't solve the problem you think it
solves, and so is a more complicated solution to the problem is does solve. I
may be wrong, though.

For various reasons, I'm actually going to end up being back towards the middle
of Jan, not the end. Trudelle is going to arrange to do triage on any dirviewer
bugs which come up while I'm away, so they will be looked at, but will probably
have a low priority. That doesn't stop someone else from fixing them, of course :)

I'm going to attach an untested patch which may help with the i18n problem. IT
may not, since I can't test.

Comment 18

[Bradley Baetz (:bbaetz)]

Attachment: possible fix?

Comment 19

[Chak Nanga]

Regarding i18n issues.....

We have two issues here:

1. What bbaetz it talking about - some i18n chars do not display correctly in
the html dir list
2. When trying to test this i found another issue - some files (with unicode
chars in their file names) do not get listed at all in the html dir listing.
[This happens in both the html dir display or in the tree widget UI]

To reproduce the second issue:
 - Create a file named c:\1/83/87/8.txt
   [Note that the "1/8", "2/8", "7/8" are actually *single*chars represented in
the unicode charset and not a char "1" followed by a "/" and "8" and so on.
Please see below on the note from Naoki on how to get unicode char filenames]

- Create a file named c:\junk.txt

- Do a file:///c:/

- You will see "junk.txt" in the listing and not "1/83/87/8.txt"
Here's my findings on the second issue so far:
==============================================
When the dir index stream code goes to enumerate the contents of the dir we
eventually end up in nspr's _PR_MD_READ_DIR() to enumerate the files:
http://lxr.mozilla.org/seamonkey/source/nsprpub/pr/src/md/windows/w95io.c#534

During enumeration |FindNextFile(d->d_hdl, &(d->d_entry)| returns "???.txt" for
the filename in |d->d_entry.cFileName| - when it encounters "c:\1/83/87/8.txt".

Eventually, when the nsDirEnumerator::HasMoreElements() checks for the file's
precense at http://lxr.mozilla.org/seamonkey/source/xpcom/io/nsLocalFileWin.cpp#201
it will return negative since there's no file named "???.txt" and hence does not
get included in the list of files.

Cc:ing Ftang/Naoki for thier input....

Creating file names with UniCode chars:
=======================================
1. Go to the "CharacterMap" accessory.
2. Click on the "Advanced Window" check box
3. Choose the "Lucida Sans Unicode" font in the "Font" list
4. Type "215B" in the  "Go to Unicode" field.
    - This should hilight the "1/8" char. Click "Select"
    - Similarly hilight the "3/8" and "7/8" chars and choose "Select"
    - Click "Copy" to copy the string to the clipboard
5. Open Notepad, type some text and choose "Save As"
6. For the filename in the SaveAs dlg paste the previously copied name from
clipboard.

Comment 20

[Bradley Baetz (:bbaetz)]

This will only work on a non en-US version of W2K. theres another bug on us
calling teh wrong windows API calls, or something like that.

What version of windows are you testing on?

Comment 21

[Chak Nanga]

I'm testing on an English version of Win2K Pro.

Comment 22

[Chak Nanga]

->bbaetz

Comment 23

[Bradley Baetz (:bbaetz)]

You can't test on the english version; it won't work. See bug 85836 and friends.

Comment 24

[Bradley Baetz (:bbaetz)]

This is basically the same as bug 102812.

*** This bug has been marked as a duplicate of 102812 ***

Comment 25

[benc]

VERIFIED: