Last Comment Bug 749500 - Bug 731878 breaks SDK 10.5 build
: Bug 731878 breaks SDK 10.5 build
Product: Core
Classification: Components
Component: Widget: Cocoa (show other bugs)
: Trunk
: x86 Mac OS X
-- normal (vote)
: mozilla15
Assigned To: Masayuki Nakano [:masayuki]
: Markus Stange [:mstange]
Depends on:
Blocks: 731878
  Show dependency treegraph
Reported: 2012-04-26 20:49 PDT by Makoto Kato [:m_kato]
Modified: 2012-04-30 08:02 PDT (History)
5 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch (1006 bytes, patch)
2012-04-27 06:01 PDT, Masayuki Nakano [:masayuki]
smichaud: review+
paul: feedback-
Details | Diff | Splinter Review
Patch (1.26 KB, patch)
2012-04-27 18:45 PDT, Masayuki Nakano [:masayuki]
smichaud: review+
Details | Diff | Splinter Review

Description User image Makoto Kato [:m_kato] 2012-04-26 20:49:04 PDT
Now, thunderbird trunk is burning due to landing bug 731878.  Is this API from 10.6?

/builds/slave/macosx-comm-central-bloat/build/mozilla/widget/cocoa/ warning: 'NSEvent' may not respond to '+pressedMouseButtons'
/builds/slave/macosx-comm-central-bloat/build/mozilla/widget/cocoa/ warning: (Messages without a matching method signature
/builds/slave/macosx-comm-central-bloat/build/mozilla/widget/cocoa/ warning: will be assumed to return 'id' and accept
/builds/slave/macosx-comm-central-bloat/build/mozilla/widget/cocoa/ warning: '...' as arguments.)
/builds/slave/macosx-comm-central-bloat/build/mozilla/widget/cocoa/ error: invalid conversion from 'objc_object*' to 'NSUInteger'
Comment 1 User image Masayuki Nakano [:masayuki] 2012-04-26 20:51:48 PDT
Yes. The API was introduced on 10.6.
Comment 2 User image Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2012-04-26 22:48:49 PDT
(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) || \
> @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 User image Mark Banner (:standard8) 2012-04-26 23:13:00 PDT
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 User image Masayuki Nakano [:masayuki] 2012-04-27 06:01:31 PDT
Created attachment 619003 [details] [diff] [review]


Thank you, it works fine!
Comment 5 User image Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2012-04-27 07:36:39 PDT
Comment on attachment 619003 [details] [diff] [review]

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 User image Steven Michaud [:smichaud] (Retired) 2012-04-27 08:03:34 PDT
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 User image Masayuki Nakano [:masayuki] 2012-04-27 18:45:02 PDT
Created attachment 619235 [details] [diff] [review]
Comment 8 User image Steven Michaud [:smichaud] (Retired) 2012-04-29 09:54:59 PDT
Comment on attachment 619235 [details] [diff] [review]

Looks fine to me.
Comment 9 User image Masayuki Nakano [:masayuki] 2012-04-29 16:44:30 PDT

Thank you. And sorry for my fault.

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