Bug in (modules) juce_mac_FileChooser.mm?


#1

In showPlatformDialog there is the following:

   #if defined (MAC_OS_X_VERSION_10_6) && MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_6
    [panel setDirectoryURL: [NSURL fileURLWithPath: juceStringToNS (directory)]];
    [panel setNameFieldStringValue: juceStringToNS (filename)];

    if ([panel runModal] == NSOKButton)
   #else
    if ([panel runModalForDirectory: juceStringToNS (directory)
                               file: juceStringToNS (filename)] == NSOKButton)
   #endif

But it seems like the first line should be:

   #if defined (MAC_OS_X_VERSION_10_6) && MAC_OS_X_VERSION_MIN_ALLOWED >= MAC_OS_X_VERSION_10_6

Otherwise, the dialog won’t appear when building for, say, 10.5 with the newer SDK, since setDirectoryURL will throw an uncaught exception.


#2

Ah… Yes, I think you’re right, thanks for the heads-up!

(Those OSX version macros always confuse me…)


#3

…although of course it’s not quite that simple. The old method is actually deprecated, and produces warnings, so in fact I think it’ll need to dynamically check which method is available on the current OS. PITA.


#4

It actually builds clean. You only get a deprecated warning if the MIN is set to 10.6 or 10.7 and you try to use the old call. In this case, you are not trying to compile those calls unless the deployment version matches.

FWIW, those OS defines have always made my head throb. Same with the platform defines in the Windows DDK.


#5

P.S. It definitely doesn’t work on 10.5.8 without the change. The dialog explodes and the app sits there with the alternate menu in place.


#6

This is how I think it should look… Works fine for me on 10.7 but if you have an old 10.5 I’d be keen to hear whether it runs ok.

[code] NSInteger result;

#if defined (MAC_OS_X_VERSION_10_6) && MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_6
if ([panel respondsToSelector: @selector (setDirectoryURL:)])
{
[panel setDirectoryURL: [NSURL fileURLWithPath: juceStringToNS (directory)]];
[panel setNameFieldStringValue: juceStringToNS (filename)];

    result = [panel runModal];
}
else
{
    // (dynamic invocation to avoid a deprecation warning)
    result = (NSInteger) [panel performSelector: @selector (runModalForDirectory:file:)
                                     withObject: juceStringToNS (directory)
                                     withObject: juceStringToNS (filename)];
}

#else
{
result = [panel runModalForDirectory: juceStringToNS (directory)
file: juceStringToNS (filename)];
}
#endif

if (result == NSOKButton)
{
    if (isSaveDialogue)

[/code]


#7

Sorry, I should have looked closer at the original line. I think you want this:

   #if defined (MAC_OS_X_VERSION_10_6) && (MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_6)
    [panel setDirectoryURL: [NSURL fileURLWithPath: juceStringToNS (directory)]];
    [panel setNameFieldStringValue: juceStringToNS (filename)];
    
    if ([panel runModal] == NSOKButton)
   #else
    if ([panel runModalForDirectory: juceStringToNS (directory)
                               file: juceStringToNS (filename)] == NSOKButton)
   #endif

Notice that I didn’t just change MAX to MIN, but also ALLOWED to REQUIRED. The problem is that when the deployment target is the same as the base SDK, MAC_OX_X_VERSION_MIN_ALLOWED doesn’t get defined. So in your case, the #if was failing and you were getting the #else, which is wrong for 10.7, hence the warning. I had the deployment target set to 10.5, so I built clean and got the old style call, even with the 10.7 SDK building on Lion.

With the above, A deployment target of 10.6 and above gets the new flavor, below gets the old. Both should build without warnings (they do here).

I won’t have access to a 10.5 machine again until Monday to try your code, but it seems like it would always run the dynamic test version when building with SDKs 10.6 or newer and only hit the #else case when building with older SDKs.

Sorry I missed the ALLOWED earlier. That’s bit me before.


#8

P.S. The () around the compare in the #if line is an old habit with the Apple stuff. Back at 10.2, the preprocessor precedence order was whacked.


#9

Ok, thanks, I’ll have another think about that.


#10

NP

FWIW, I get why Apple has the _ALLOWED thing, since folks will compile their code against different SDKs. But something about the _REQUIRED naming, which derives directly from deployment target always confuses me, and I’ve read the Apple guide multiple times.