Last Comment Bug 753256 - Cocoa widget shouldn't use GetCurrentKeyModifiers() directly
: Cocoa widget shouldn't use GetCurrentKeyModifiers() directly
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Widget: Cocoa (show other bugs)
: Trunk
: All Mac OS X
: -- normal (vote)
: mozilla15
Assigned To: Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured)
:
Mentors:
Depends on:
Blocks: 630811 731878
  Show dependency treegraph
 
Reported: 2012-05-09 00:30 PDT by Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured)
Modified: 2012-05-16 03:32 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (3.92 KB, patch)
2012-05-10 02:32 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured)
no flags Details | Diff | Review
Patch (altenative) (3.97 KB, patch)
2012-05-15 00:51 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured)
smichaud: review+
Details | Diff | Review

Description Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-05-09 00:30:03 PDT
Currently, GetCurrentKeyModifiers() is used for getting *cocoa* modifier flags. But the value is different from expected value.

I'll post a patch soon.
Comment 1 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-05-10 02:32:57 PDT
Created attachment 622661 [details] [diff] [review]
Patch

I think that we should discuss whether we should use event-queue-synthesized API or not. But for now, we should fix just the bug.
Comment 2 Steven Michaud [:smichaud] (Retired) 2012-05-14 14:17:15 PDT
This looks fine to me.

But don't you also need to handle 'rightShiftKey', 'rightOptionKey' and 'rightControlKey' in nsCocoaUtils::GetCurrentModifiers()?
Comment 3 Steven Michaud [:smichaud] (Retired) 2012-05-14 14:18:07 PDT
> event-queue-synthesized

You mean event-queue-synchronized?
Comment 4 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-05-15 00:04:42 PDT
(In reply to Steven Michaud from comment #2)
> This looks fine to me.
> 
> But don't you also need to handle 'rightShiftKey', 'rightOptionKey' and
> 'rightControlKey' in nsCocoaUtils::GetCurrentModifiers()?

I've never known the constants. However, looks like ::GetCurrentKeyModifiers() doesn't return the values. On my environment, both sides modifiers cause same result.
Comment 5 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-05-15 00:05:01 PDT
(In reply to Steven Michaud from comment #3)
> > event-queue-synthesized
> 
> You mean event-queue-synchronized?

Ah, yes.
Comment 6 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-05-15 00:51:13 PDT
Created attachment 623964 [details] [diff] [review]
Patch (altenative)

If you like this better, please r+ this.
Comment 7 Steven Michaud [:smichaud] (Retired) 2012-05-15 13:47:22 PDT
>> But don't you also need to handle 'rightShiftKey', 'rightOptionKey' and
>> 'rightControlKey' in nsCocoaUtils::GetCurrentModifiers()?
>
> I've never known the constants.

They're documented here (among other places):

http://developer.apple.com/legacy/mac/library/documentation/Carbon/Reference/Event_Manager/Reference/reference.html#//apple_ref/doc/uid/TP30000158-CH3g-C004232
Comment 8 Steven Michaud [:smichaud] (Retired) 2012-05-15 13:47:55 PDT
Comment on attachment 623964 [details] [diff] [review]
Patch (altenative)

Looks fine to me.
Comment 9 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-05-15 17:30:04 PDT
Comment on attachment 622661 [details] [diff] [review]
Patch

https://hg.mozilla.org/integration/mozilla-inbound/rev/72349c11ceaa
Comment 10 Ed Morley [:emorley] 2012-05-16 03:32:19 PDT
https://hg.mozilla.org/mozilla-central/rev/72349c11ceaa

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