Last Comment Bug 678423 - We need to conditionally define the 10.7 Cocoa symbols for 10.6 SDK and lower.
: We need to conditionally define the 10.7 Cocoa symbols for 10.6 SDK and lower.
Status: VERIFIED FIXED
:
Product: Core
Classification: Components
Component: Widget: Cocoa (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: ---
Assigned To: Steven Michaud [:smichaud] (Retired)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-11 18:34 PDT by Zibi Braniecki [:gandalf][:zibi]
Modified: 2011-08-12 13:37 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fix (2.47 KB, patch)
2011-08-12 09:29 PDT, Steven Michaud [:smichaud] (Retired)
b56girard: review+
Details | Diff | Splinter Review

Description Zibi Braniecki [:gandalf][:zibi] 2011-08-11 18:34:34 PDT
Spin off of bug 668953 (see bug 668953 comment 91)

 - we'll need to define 10.7 SDK cocoa symbols when using the 10.6 SDK or lower.
 - we need a way to tell which SDK we're using in compiled
code
Comment 1 Markus Stange [:mstange] 2011-08-12 07:54:50 PDT
(In reply to Zbigniew Braniecki [:gandalf] from comment #0)
>  - we need a way to tell which SDK we're using in compiled code

MAC_OS_X_VERSION_MAX_ALLOWED is exactly that, even though it's very confusingly named. We already use it, for example here:
http://mxr.mozilla.org/mozilla-central/source/widget/src/cocoa/nsCocoaWindow.h#138

Using 
#if !defined(MAC_OS_X_VERSION_10_7) || MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
instead of
#ifdef __LP64__
for the enum and interface definitions in nsChildView.h would be exactly the right thing to do here.
Comment 2 Steven Michaud [:smichaud] (Retired) 2011-08-12 07:58:40 PDT
I now agree, Benoit.  I'm about to try it out with a 10.7 SDK build on OS X 10.7.

If that works (which it should), I'll post a patch and ask you to review.
Comment 3 Steven Michaud [:smichaud] (Retired) 2011-08-12 08:02:02 PDT
> I now agree, Benoit.

Oops.  I now agree with both Benoit (bug 668963 comment #98) and Markus.
Comment 4 Steven Michaud [:smichaud] (Retired) 2011-08-12 09:29:35 PDT
Created attachment 552673 [details] [diff] [review]
Fix
Comment 5 Steven Michaud [:smichaud] (Retired) 2011-08-12 09:51:22 PDT
Comment on attachment 552673 [details] [diff] [review]
Fix

Landed on mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/5c8b3f1db588
Comment 6 Steven Michaud [:smichaud] (Retired) 2011-08-12 10:08:04 PDT
> instead of
> #ifdef __LP64__

As long as we continue to use the 10.5 SDK for 32-bit builds, we can't
use the patch for bug 668953 in 32-bit builds -- the code compiles on
10.6, but won't link.  That's why I used #ifdef __LP64__.

Benoit suggested we might use MAC_OS_X_VERSION_MIN instead.  But I'd
feel obliged to double-check the docs and do test compiles, and we
don't have time for that.  I've	been using __LP64__ all along, and
know that it works.

If we haven't replaced __LP64__ by the time we stop using the 10.5
SDK, we can just drop it -- it'll no	longer be needed.

There's been talk of dropping the 10.5 SDK even before we drop support
for running on OS X 10.5.  See bug 674655.  But this might not work
out.
Comment 7 Markus Stange [:mstange] 2011-08-12 12:30:46 PDT
(In reply to Steven Michaud from comment #6)
> > instead of
> > #ifdef __LP64__
> 
> As long as we continue to use the 10.5 SDK for 32-bit builds, we can't
> use the patch for bug 668953 in 32-bit builds -- the code compiles on
> 10.6, but won't link.

Which part exactly? And why? Have you tested it?
Sorry to be so inquiring, but I'd really like to understand this :)
Comment 8 Steven Michaud [:smichaud] (Retired) 2011-08-12 12:42:21 PDT
It has to do with "blocks".  See bug 668953 comment #67.
Comment 9 Markus Stange [:mstange] 2011-08-12 12:59:58 PDT
Oh, right. I had forgotten that the interface definition has blocks, too (and not only the chunk of code in nsChildView.mm). OK, thanks.
Comment 10 Zibi Braniecki [:gandalf][:zibi] 2011-08-12 13:37:59 PDT
v.

Note You need to log in before you can comment on or make changes to this bug.