Last Comment Bug 749500 - Bug 731878 breaks SDK 10.5 build
: Bug 731878 breaks SDK 10.5 build
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Widget: Cocoa (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla15
Assigned To: Masayuki Nakano [:masayuki] (Mozilla Japan)
:
Mentors:
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:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


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

Description 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?


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] (Mozilla Japan) 2012-04-26 20:51:48 PDT
Yes. The API was introduced on 10.6.
Comment 2 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) || \
>     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) 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 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-04-27 06:01:31 PDT
Created attachment 619003 [details] [diff] [review]
Patch

zpao:

Thank you, it works fine!
Comment 5 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2012-04-27 07:36:39 PDT
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) 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 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-04-27 18:45:02 PDT
Created attachment 619235 [details] [diff] [review]
Patch
Comment 8 Steven Michaud [:smichaud] (Retired) 2012-04-29 09:54:59 PDT
Comment on attachment 619235 [details] [diff] [review]
Patch

Looks fine to me.
Comment 9 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-04-29 16:44:30 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/baa40113b2d2

Thank you. And sorry for my fault.
Comment 10 :Ehsan Akhgari (out sick) 2012-04-30 08:02:05 PDT
https://hg.mozilla.org/mozilla-central/rev/baa40113b2d2

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