236844 - [beos] search for helper apps in mozilla directory before $PATH

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect)

Description

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

Beos version of bug 229636

the new expectation for GetFileTokenFromPath is that it searches in the mozilla
directory, too.

Comment 1

[Niels Reedijk]

Attachment: Remove our implementation and rely on the base implementation

I think this will solve all the issues.

The old behaviour was: enter the path in BPath and it would take absolute and
relative paths, on the latter translate this one into a absolute path, and
check whether or not the file exists. This means that our implementation
already did what bug 229636 tried to do. 

The behaviour of the new GetFileTokenforPath implementation is as follows:
a) check if it is an absolute path. By feeding the path into NS_NewLocalFile
you automatically rule out relative paths, because the internals of that
function only recognise paths starting with '~/' and with '/'. 
b) check if the file exists in the mozilla directory. The entered path is
appended to the path of the mozilla directory. Then it is ran through
NS_NewLocalFile (which succeeds) and then is checked by NSIFile::Exists(). The
BeOS implementation runs this through BPath so we in fact get the
implementation found in our old implementation.

One note remains: the initial comment mentions something about PATH, but since
the old implementation never looks at PATH, the new one doesn't have to do that
either. 

So, we can remove our implementation and rely on the provided one, which (a bit
less efficient) does the same. In order to reduce code complexity, I vote this
sollution :-). 

P.S. Any way to test this?

Comment 2

[tqh]

I'll vote for that version too, if it works :)

Comment 3

[Doug Shelton]

I've installed the patch but don't know a test methodology.  Would it be
relevant to install the general coffee flash player with plugin in the firefox
plugin directory?

Comment 4

[Niels Reedijk]

Comment on attachment 196633 [details] [diff] [review]
Remove our implementation and rely on the base implementation

As it doesn't seem to affect anything running with it succesfully and no
problem opening things), can this bug be committed?

Comment 5

[Sergei Dolgov]

I probably will test this patch in next two days.
Actually i had look in that part of code, but that was long ago. So ath the
moment i remember nothing:(
How did you test it?
With plugins?
With Reveal from download manager?
With autoopening for example *.rtf with Gobe and *.pdf with BePDF when clicking
on links?

Comment 6

[Niels Reedijk]

(In reply to comment #5)
> I probably will test this patch in next two days.
> Actually i had look in that part of code, but that was long ago. So ath the
> moment i remember nothing:(

It's not at all that intimidating (at least, this part of the changes), so
you'll get the hang of it pretty fast.

> How did you test it?

Okay, I'm currently checking where exactly the function is called. Okay,
http://lxr.mozilla.org/mozilla/ident?i=GetFileTokenForPath . So it seems that it
is called all over the place, meaning that if it didn't work, breakage would be
all over the place. But it does need some testing, naturally.

> With plugins?

Unfortunately, I have none. Anything you could point me to?

> With Reveal from download manager?

Just tried it: it works flawlessly. Reveal, open and the moz_icon patch all work
like expected: they show/open the file.

> With autoopening for example *.rtf with Gobe and *.pdf with BePDF when clicking
> on links?

Well, it doesn't auto-open, but I'm not sure it did before this patch. It does
start the open dialog and BePDF as default.

So at the moment nothing seems broken yet.

Comment 7

[Niels Reedijk]

2005-09-26 21:51	timeless%mozdev.org 	mozilla/ uriloader/ exthandler/ beos/
nsOSHelperAppService.cpp 	1.14 	0/28  	Bug 236844 [beos] search for helper apps
in mozilla directory before $PATH patch by [e-mail] r=sergei_d

Committed in HEAD

Comment 8

[Niels Reedijk]

Comment on attachment 196633 [details] [diff] [review]
Remove our implementation and rely on the base implementation

Approval requested. This is a BeOS only change (in a BeOS-only location), and
it will therefore not affect any other build, so it is a safe patch. Tested and
verified here.