Closed Bug 753256 Opened 8 years ago Closed 8 years ago

Cocoa widget shouldn't use GetCurrentKeyModifiers() directly

Categories

(Core :: Widget: Cocoa, defect)

All
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

Attachments

(1 file, 1 obsolete file)

Currently, GetCurrentKeyModifiers() is used for getting *cocoa* modifier flags. But the value is different from expected value.

I'll post a patch soon.
Attached patch Patch (obsolete) — Splinter Review
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.
Attachment #622661 - Flags: review?(smichaud)
This looks fine to me.

But don't you also need to handle 'rightShiftKey', 'rightOptionKey' and 'rightControlKey' in nsCocoaUtils::GetCurrentModifiers()?
> event-queue-synthesized

You mean event-queue-synchronized?
(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.
(In reply to Steven Michaud from comment #3)
> > event-queue-synthesized
> 
> You mean event-queue-synchronized?

Ah, yes.
If you like this better, please r+ this.
Attachment #623964 - Flags: review?(smichaud)
>> 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 on attachment 623964 [details] [diff] [review]
Patch (altenative)

Looks fine to me.
Attachment #623964 - Flags: review?(smichaud) → review+
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/72349c11ceaa
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.