Closed
Bug 753256
Opened 12 years ago
Closed 12 years ago
Cocoa widget shouldn't use GetCurrentKeyModifiers() directly
Categories
(Core :: Widget: Cocoa, defect)
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: masayuki, Assigned: masayuki)
References
Details
Attachments
(1 file, 1 obsolete file)
3.97 KB,
patch
|
smichaud
:
review+
|
Details | Diff | Splinter Review |
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•12 years ago
|
||
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)
Comment 2•12 years ago
|
||
This looks fine to me. But don't you also need to handle 'rightShiftKey', 'rightOptionKey' and 'rightControlKey' in nsCocoaUtils::GetCurrentModifiers()?
Comment 3•12 years ago
|
||
> event-queue-synthesized
You mean event-queue-synchronized?
Assignee | ||
Comment 4•12 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•12 years ago
|
||
(In reply to Steven Michaud from comment #3) > > event-queue-synthesized > > You mean event-queue-synchronized? Ah, yes.
Assignee | ||
Comment 6•12 years ago
|
||
If you like this better, please r+ this.
Attachment #623964 -
Flags: review?(smichaud)
Comment 7•12 years ago
|
||
>> 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•12 years ago
|
||
Comment on attachment 623964 [details] [diff] [review] Patch (altenative) Looks fine to me.
Attachment #623964 -
Flags: review?(smichaud) → review+
Assignee | ||
Comment 9•12 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•12 years ago
|
Target Milestone: --- → mozilla15
Comment 10•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/72349c11ceaa
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•