122571 - Need API to detect a Mac OS 9/X package

Status: VERIFIED FIXED

(Core Graveyard :: File Handling, defect)

Description

[Robert John Churchill]

Comment 1

[Robert John Churchill]

Attachment: IsPackage() method

Comment 2

[Robert John Churchill]

This patch adds a IsPackage() method onto nsILocalFileMac

Conrad, care to review?

Comment 3

[Conrad Carlen (not reading bugmail)]

Comment on attachment 67068 [details] [diff] [review]
IsPackage() method

>+NS_IMETHODIMP  
>+nsLocalFile::IsPackage(PRBool *outIsPackage)
>+{
>+    NS_ENSURE_ARG(outIsPackage);
>+    *outIsPackage = PR_FALSE;
>+
>+    PRBool isDir;
>+    nsresult rv = IsDirectory(&isDir); // Calls ResolveAndStat()
>+    if (NS_FAILED(rv)) return rv;

Since IsDirectory() also calls UpdateCachedCatInfo(), I'd change the comment to
say that and not make another call to UpdateCachedCatInfo(TRUE). Unless you
really want to pass TRUE and force an update of the cached info.

>+    
>+    rv = UpdateCachedCatInfo(PR_TRUE);
>+    if (NS_FAILED(rv)) return rv;
>+

>+            const char *extPtr = strrchr(name, '.');
>+            if (extPtr)
>+            {
>+                if (!nsCRT::strcasecmp(extPtr, ".app"))

There's also .framework and .bundle. Do we want to consider those as well?
Probably.

>+                {
>+                    *outIsPackage = PR_TRUE;
>+                }
>+            }
>+        }
>+    }
>+
>+    return NS_OK;
>+}
>+

Other than those points, which are debatable, looks good.

Comment 4

[Ludovic Hirlimann]

How do you intend to support BSD utils like bzip2/gzip in X ?

Comment 5

[Robert John Churchill]

> Since IsDirectory() also calls UpdateCachedCatInfo(), I'd change the comment
> to say that and not make another call to UpdateCachedCatInfo(TRUE).

OK.


> There's also .framework and .bundle. Do we want to consider those as well?

I think the answer is "no", after observing how the Finder treats folders with
those extensions (it doesn't consider them to be a single item).

Comment 6

[Robert John Churchill]

Attachment: Patch which just uses IsDirectory()

Patch incorporating Conrad's feedback [just use IsDirectory()]

Comment 7

[Conrad Carlen (not reading bugmail)]

Comment on attachment 67125 [details] [diff] [review]
Patch which just uses IsDirectory()

Yeah, if the Finder only treats .app differently and treats .bundle as a
directory, that's the way to go.
r=ccarlen.

Comment 8

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 67125 [details] [diff] [review]
Patch which just uses IsDirectory()

sr=[e-mail]

Comment 9

[Robert John Churchill]

Fixed.

Comment 10

[Chris Petersen]

Verified..