173099 - GetFileTokenForPath() is declared in base class instead port-implementations

Status: VERIFIED WONTFIX

(Core Graveyard :: File Handling, defect, P4)

Description

[Sergei Dolgov]

GetFileTokenForPath() in uriloader/exthandler is declared in base class
(uriloader/exthandler/nsExternalHelperAppService.h) instead port-implementations
It reduces flexibility and causes (re)conversion overheads sometimes - see
comments to bug 145860 for explanation and details
http://bugzilla.mozilla.org/show_bug.cgi?id=145860#c33

Hope Boris will set proper component for this bug.

Comment 1

[Sergei Dolgov]

setting weak dependency :)

Comment 2

[Boris Zbarsky [:bzbarsky]]

I'll try to get to this sometime, but it won't be for a few weeks... so don't 
hold up bug 145860 for this.  ;)

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

so... how is GetFileTokenForPath different from NS_NewLocalFile?

Comment 4

[Boris Zbarsky [:bzbarsky]]

It resolves relative to each component of $PATH in turn until it finds a file. 
So if $PATH == /bin:/usr/bin:/sbin and the path is "emacs" it would come out
with an nsIFile pointing to /usrb/bin/emacs

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

ok, but in that case, isn't it needed by GetMIMEInfoFromDS?

Comment 6

[Boris Zbarsky [:bzbarsky]]

No, GetMIMEInfoFromDS only handles absolute paths... See bug 56662.  Since on 
the Mac the data stored in the DS is not a path, really, things are hard.

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

hm, so what is
http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/nsExternalHelperAppService.cpp#559
for?

Comment 8

[Boris Zbarsky [:bzbarsky]]

Hrm.  Looks like a Mac bug in waiting to me unless GetFileTokenForPath does 
something clever on mac... 

Odd, though.  Given that code, whence bug 56662?

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

hm... per
http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/mac/nsOSHelperAppService.cpp#173
the mac code doesn't do anything clever, just does an InitWithPath on a new
ILocalFile...

>Odd, though.  Given that code, whence bug 56662?
see comment in that bug

Comment 10

[Boris Zbarsky [:bzbarsky]]

Ah, so this code is just broken-as-designed on the mac.  Fun.

Comment 11

[Steve Dagley]

ccarlen is the nsLocalFileOSX guru

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

ok. bug 166369 demonstrated that this code must be callable from
nsExternalHelperAppService (i.e. the base class) - it must have a way to get
from the RDF datasource literal to an nsILocalFile.

as this function a) searches the path (at least on unix) and b) tries both CFM
and unix paths on OSX, this can't be implemented in an xp way in the base class.

I hence disagree with bz in bug 145860 comment 33 ("no one but the child classes
should be calling it anyway"). this also means that I'd like this bug to be WONTFIX.

does someone have complaints about that?

Comment 13

[Boris Zbarsky [:bzbarsky]]

No, that's ok... Sucks for BeOS because of the string conversions involved, but
that's life.

Comment 14

[Christian :Biesinger (don't email me, ping me on IRC)]

verified

beos could of course add a private/protected version of getfiletokenforpath that
takes an utf8 string and call that from SetMIMEInfoForType (and from
GetFileTokenForPath)... but that would be another bug.