1412978 - Delete nsFileSystemDataSource as it seems to be dead code

Status: RESOLVED FIXED

(Core Graveyard :: RDF, enhancement)

Description

[Marco Castelluccio [:marco]]

From the code coverage report, it looks like this file is never executed during tests.
From a quick inspection, it looks like dead code to me.

Comment 1

[Marco Castelluccio [:marco]]

Attachment: Remove nsFileSystemDataSource

Comment 2

[Axel Hecht [:Pike]]

Marco, which inspection did you do to tell if this is dead code?

I'm asking because the lack of test coverage doesn't mean a whole lot in RDF land.

Comment 3

[Marco Castelluccio [:marco]]

(In reply to Axel Hecht [:Pike] from comment #2)
> Marco, which inspection did you do to tell if this is dead code?
> 
> I'm asking because the lack of test coverage doesn't mean a whole lot in RDF
> land.

I couldn't find any reference to the contract ID of the service, that is "@mozilla.org/rdf/datasource;1?name=files".

Comment 4

[Axel Hecht [:Pike]]

Comment on attachment 8923554 [details] [diff] [review]
Remove nsFileSystemDataSource

Review of attachment 8923554 [details] [diff] [review]:
-----------------------------------------------------------------

Actually, https://dxr.mozilla.org/comm-central/source/suite/common/directory/directory.xul#24 still uses the file system datasource.

Frank-Rainer, what's your take?

Clearing the review on me for now.

Comment 5

[Marco Castelluccio [:marco]]

If it is only used in comm-central, we can disable building it in Firefox.

Comment 6

[Frank-Rainer Grahl (:frg)]

Seems to be linked to:
 
https://dxr.mozilla.org/comm-central/source/mozilla/xpfe/components/directory

and pretty old code I don't know much about

IanN, stefanh do you know what this one does in suite?

Comment 7

[Stefan [:stefanh]]

(In reply to Frank-Rainer Grahl (:frg) from comment #6)
> IanN, stefanh do you know what this one does in suite?

It displays a directory listing as a XUL tree, see http://kb.mozillazine.org/Network.dir.format and https://dxr.mozilla.org/mozilla-central/rev/c2248f85346939d3e0b01f57276c440ccb2d16a1/xpfe/components/directory/nsDirectoryViewer.cpp#1253

Does it work? I couldn't resist trying and it still works for local directory listings. Don't know if anyone uses it, though. The default is to display dir listings in html format.

Comment 8

[Frank-Rainer Grahl (:frg)]

> Does it work?

Probably but I don't see any actual use of it in SeaMonkey. Not needed for ftp as far as I see it. Maybe this was used in add-ons a long time ago? There is also nsIHTTPIndex, nsHTTPIndex and the xpfe/components/directory/ See some references for HTTPIndex in suite but they seem to be removable too. Before we rip out code I would really like to wait for IanNs advice/remarks.

Comment 9

[Frank-Rainer Grahl (:frg)]

oops http-index-format is still needed I think :)

Comment 10

[Masatoshi Kimura [:emk]]

Network.dir.format also works with ftp directory listings. See bug 1239239.

Comment 11

[Frank-Rainer Grahl (:frg)]

Thanks. I see it now. Only used with network.dir.format set to 3.

So IanN should decide what to do. ftp access will not be broken by removing it and if I need full access I am using FileZilla anyway so fine with me either way.

FRG

Comment 12

[Ian Neal]

Comment on attachment 8923554 [details] [diff] [review]
Remove nsFileSystemDataSource

f=me

Will need to spin off bugs to deal with removing other code from both suite and xpfe and any other fallout.

Comment 13

[Axel Hecht [:Pike]]

Comment on attachment 8923554 [details] [diff] [review]
Remove nsFileSystemDataSource

Review of attachment 8923554 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, r=me.

This will create a conflict with bug 685236, but not in a bad way.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/24ad01d1f86b
Delete nsFileSystemDataSource as it is dead code. r=Pike

Comment 15

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/24ad01d1f86b

Comment 16

[Gerry Chang [:gchang]]

It's too late for 58, won't fix for 58.