Bug 93535 - [chromium/mac] Map NSEventPhaseMayBegin appropriately on 10.8
Summary: [chromium/mac] Map NSEventPhaseMayBegin appropriately on 10.8
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: asvitkine
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-08 15:08 PDT by asvitkine
Modified: 2012-11-06 11:57 PST (History)
3 users (show)

See Also:


Attachments
Patch (2.75 KB, patch)
2012-08-08 15:11 PDT, asvitkine
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description asvitkine 2012-08-08 15:08:11 PDT
[chromium/mac] Map NSEventPhaseMayBegin appropriately on 10.8
Comment 1 asvitkine 2012-08-08 15:11:50 PDT
Created attachment 157307 [details]
Patch
Comment 2 asvitkine 2012-08-08 15:14:31 PDT
Nico: Please review the change.
Dimitri: Will need you approval (assuming Nico still isn't a reviewer).
Comment 3 Nico Weber 2012-08-08 15:21:42 PDT
Comment on attachment 157307 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=157307&action=review

LGTM, thanks for doing this. (Bug 79868 has some background.)

> Source/WebKit/chromium/src/mac/WebInputEventFactory.mm:57
> +#if __MAC_OS_X_VERSION_MAX_ALLOWED < 1080

https://developer.apple.com/library/mac/#documentation/Cocoa/Reference/ApplicationKit/Classes/nsevent_Class/Reference/Reference.html doesn't mention this for 10.8 either , so maybe omit the #if completely?
Comment 4 asvitkine 2012-08-08 15:27:59 PDT
Comment on attachment 157307 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=157307&action=review

>> Source/WebKit/chromium/src/mac/WebInputEventFactory.mm:57
>> +#if __MAC_OS_X_VERSION_MAX_ALLOWED < 1080
> 
> https://developer.apple.com/library/mac/#documentation/Cocoa/Reference/ApplicationKit/Classes/nsevent_Class/Reference/Reference.html doesn't mention this for 10.8 either , so maybe omit the #if completely?

It's in the 10.8 SDK NSEvent.h header though...

Wouldn't omitting it result in a build error when building against the 10.8 SDK? (Or do compilers ignore multiply-defined enums with same values these days like they do with typedefs?)
Comment 5 Nico Weber 2012-08-08 15:32:36 PDT
(In reply to comment #4)
> (From update of attachment 157307 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=157307&action=review
> 
> >> Source/WebKit/chromium/src/mac/WebInputEventFactory.mm:57
> >> +#if __MAC_OS_X_VERSION_MAX_ALLOWED < 1080
> > 
> > https://developer.apple.com/library/mac/#documentation/Cocoa/Reference/ApplicationKit/Classes/nsevent_Class/Reference/Reference.html doesn't mention this for 10.8 either , so maybe omit the #if completely?
> 
> It's in the 10.8 SDK NSEvent.h header though...

Ah ok, then it's correct as is (I don't have a 10.8 SDK at hand to check).

> 
> Wouldn't omitting it result in a build error when building against the 10.8 SDK? (Or do compilers ignore multiply-defined enums with same values these days like they do with typedefs?)

No, multiply-defined enums are an error. I assumed the documentation would match the header. Silly me.
Comment 6 Dimitri Glazkov (Google) 2012-08-08 15:34:56 PDT
Comment on attachment 157307 [details]
Patch

rs=me
Comment 7 WebKit Review Bot 2012-08-08 17:40:52 PDT
Comment on attachment 157307 [details]
Patch

Clearing flags on attachment: 157307

Committed r125125: <http://trac.webkit.org/changeset/125125>
Comment 8 WebKit Review Bot 2012-08-08 17:40:56 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 asvitkine 2012-11-06 11:57:23 PST
For historical purposes, this fixed http://crbug.com/139553.