749500 - Bug 731878 breaks SDK 10.5 build

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Makoto Kato [:m_kato]]

Now, thunderbird trunk is burning due to landing bug 731878.  Is this API from 10.6?


http://tinderbox.mozilla.org/showlog.cgi?log=ThunderbirdTrunk/1335491470.1335491837.8282.gz#err0

/builds/slave/macosx-comm-central-bloat/build/mozilla/widget/cocoa/nsChildView.mm:3963: warning: 'NSEvent' may not respond to '+pressedMouseButtons'
/builds/slave/macosx-comm-central-bloat/build/mozilla/widget/cocoa/nsChildView.mm:3963: warning: (Messages without a matching method signature
/builds/slave/macosx-comm-central-bloat/build/mozilla/widget/cocoa/nsChildView.mm:3963: warning: will be assumed to return 'id' and accept
/builds/slave/macosx-comm-central-bloat/build/mozilla/widget/cocoa/nsChildView.mm:3963: warning: '...' as arguments.)
/builds/slave/macosx-comm-central-bloat/build/mozilla/widget/cocoa/nsChildView.mm:3963: error: invalid conversion from 'objc_object*' to 'NSUInteger'

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Yes. The API was introduced on 10.6.

Comment 2

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) (away: 4/29 - 5/6) from comment #1)
> Yes. The API was introduced on 10.6.

I think you'll want to add something like this to the header then.

> #if !defined(MAC_OS_X_VERSION_10_6) || \
>     MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_6
> @interface NSEvent (SnowLeopardEventFeatures)
> + (NSUinteger)pressedMouseButtons;
> @end
> #endif

I've also been told in the past to use [foo respondsToSelector:whatever] instead of OnVersionOrLater (feature detect instead of version check), but I think we're pretty mixed in there and that wouldn't be the issue.

Comment 3

[Mark Banner (:standard8)]

The Thunderbird builders are going to be upgrading to 10.7 with an 10.6 sdk real soon (like next couple of weeks all being well), so I don't mind too much if this is broken there.

Might want to consider if we're breaking developers using the 10.5 sdk though (I can't remember what has been said about sdk requirements).

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch

zpao:

Thank you, it works fine!

Comment 5

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Comment on attachment 619003 [details] [diff] [review]
Patch

Glad it's working! We do that sort of thing in a few other places as well. And on that note, we've kept other instances of this pattern to header files so my gut tells me we should do the same here, but I haven't spent that much time in this code. Steven will correct me if I'm wrong though!

Comment 6

[Steven Michaud [:smichaud] (Retired)]

I read Paul's comment (comment #5) after I did the review :-(

Your patch works fine, Masayuki.  But now I also think it might be better to put it into a header file.  For example there's an NSEvent (Undocumented) category in nsChildView.h to which a definition of +[NSEvent pressedMouseButtons] might be added, with or without the version-specific ifdefs.

It's not a big deal.  Just a matter of style.

The definition of +[NSEvent pressedMouseButtons] is (of course) needed when building on versions of the OS (or the SDK) that don't define it, so that you don't get compile errors (or warnings).  You still also need to only call this method on versions of the OS that support it.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch

Comment 8

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 619235 [details] [diff] [review]
Patch

Looks fine to me.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/baa40113b2d2

Thank you. And sorry for my fault.

Comment 10

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/baa40113b2d2