The default bug view has changed. See this FAQ.

Cocoa widget shouldn't use GetCurrentKeyModifiers() directly

RESOLVED FIXED in mozilla15

Status

()

Core
Widget: Cocoa
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

Trunk
mozilla15
All
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

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

I'll post a patch soon.
(Assignee)

Comment 1

5 years ago
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.
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?
(Assignee)

Comment 4

5 years ago
(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.
(Assignee)

Comment 5

5 years ago
(In reply to Steven Michaud from comment #3)
> > event-queue-synthesized
> 
> You mean event-queue-synchronized?

Ah, yes.
(Assignee)

Comment 6

5 years ago
Created attachment 623964 [details] [diff] [review]
Patch (altenative)

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+
(Assignee)

Comment 9

5 years ago
Comment on attachment 622661 [details] [diff] [review]
Patch

https://hg.mozilla.org/integration/mozilla-inbound/rev/72349c11ceaa
Attachment #622661 - Attachment is obsolete: true
Attachment #622661 - Flags: review?(smichaud)
(Assignee)

Updated

5 years ago
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/72349c11ceaa
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.