Bug 1100862 Shortcut key handlers should ignore Windows-Logo key state if no shortcut keys match a key event and the key is pressed because native applications do so r=enndeakin

This commit is contained in:
Masayuki Nakano 2014-12-12 21:17:37 +09:00
parent fd6ab332cc
commit 1a1ed7764e
7 changed files with 143 additions and 56 deletions

View File

@ -12,6 +12,7 @@
#include "nsContentUtils.h"
#include "mozilla/dom/Event.h" // for nsIDOMEvent::InternalDOMEvent()
#include "mozilla/dom/EventTarget.h"
#include "mozilla/TextEvents.h"
using namespace mozilla;
using namespace mozilla::dom;
@ -83,27 +84,40 @@ nsXBLKeyEventHandler::~nsXBLKeyEventHandler()
NS_IMPL_ISUPPORTS(nsXBLKeyEventHandler, nsIDOMEventListener)
bool
nsXBLKeyEventHandler::ExecuteMatchedHandlers(nsIDOMKeyEvent* aKeyEvent,
uint32_t aCharCode,
bool aIgnoreShiftKey)
nsXBLKeyEventHandler::ExecuteMatchedHandlers(
nsIDOMKeyEvent* aKeyEvent,
uint32_t aCharCode,
const IgnoreModifierState& aIgnoreModifierState)
{
bool trustedEvent = false;
aKeyEvent->GetIsTrusted(&trustedEvent);
WidgetEvent* event = aKeyEvent->GetInternalNSEvent();
nsCOMPtr<EventTarget> target = aKeyEvent->InternalDOMEvent()->GetCurrentTarget();
bool executed = false;
for (uint32_t i = 0; i < mProtoHandlers.Length(); ++i) {
nsXBLPrototypeHandler* handler = mProtoHandlers[i];
bool hasAllowUntrustedAttr = handler->HasAllowUntrustedAttr();
if ((trustedEvent ||
if ((event->mFlags.mIsTrusted ||
(hasAllowUntrustedAttr && handler->AllowUntrustedEvents()) ||
(!hasAllowUntrustedAttr && !mIsBoundToChrome && !mUsingContentXBLScope)) &&
handler->KeyEventMatched(aKeyEvent, aCharCode, aIgnoreShiftKey)) {
handler->KeyEventMatched(aKeyEvent, aCharCode, aIgnoreModifierState)) {
handler->ExecuteHandler(target, aKeyEvent);
executed = true;
}
}
#ifdef XP_WIN
// Windows native applications ignore Windows-Logo key state when checking
// shortcut keys even if the key is pressed. Therefore, if there is no
// shortcut key which exactly matches current modifier state, we should
// retry to look for a shortcut key without the Windows-Logo key press.
if (!executed && !aIgnoreModifierState.mOS) {
WidgetKeyboardEvent* keyEvent = event->AsKeyboardEvent();
if (keyEvent && keyEvent->IsOS()) {
IgnoreModifierState ignoreModifierState(aIgnoreModifierState);
ignoreModifierState.mOS = true;
return ExecuteMatchedHandlers(aKeyEvent, aCharCode, ignoreModifierState);
}
}
#endif
return executed;
}
@ -129,14 +143,17 @@ nsXBLKeyEventHandler::HandleEvent(nsIDOMEvent* aEvent)
nsContentUtils::GetAccelKeyCandidates(key, accessKeys);
if (accessKeys.IsEmpty()) {
ExecuteMatchedHandlers(key, 0, false);
ExecuteMatchedHandlers(key, 0, IgnoreModifierState());
return NS_OK;
}
for (uint32_t i = 0; i < accessKeys.Length(); ++i) {
IgnoreModifierState ignoreModifierState;
ignoreModifierState.mShift = accessKeys[i].mIgnoreShift;
if (ExecuteMatchedHandlers(key, accessKeys[i].mCharCode,
accessKeys[i].mIgnoreShift))
ignoreModifierState)) {
return NS_OK;
}
}
return NS_OK;
}

View File

@ -15,6 +15,12 @@ class nsIAtom;
class nsIDOMKeyEvent;
class nsXBLPrototypeHandler;
namespace mozilla {
namespace dom {
struct IgnoreModifierState;
} // namespace dom
} // namespace mozilla
class nsXBLEventHandler : public nsIDOMEventListener
{
public:
@ -48,6 +54,8 @@ private:
class nsXBLKeyEventHandler : public nsIDOMEventListener
{
typedef mozilla::dom::IgnoreModifierState IgnoreModifierState;
public:
nsXBLKeyEventHandler(nsIAtom* aEventType, uint8_t aPhase, uint8_t aType);
@ -95,7 +103,7 @@ private:
virtual ~nsXBLKeyEventHandler();
bool ExecuteMatchedHandlers(nsIDOMKeyEvent* aEvent, uint32_t aCharCode,
bool aIgnoreShiftKey);
const IgnoreModifierState& aIgnoreModifierState);
nsTArray<nsXBLPrototypeHandler*> mProtoHandlers;
nsCOMPtr<nsIAtom> mEventType;

View File

@ -603,9 +603,10 @@ nsXBLPrototypeHandler::GetController(EventTarget* aTarget)
}
bool
nsXBLPrototypeHandler::KeyEventMatched(nsIDOMKeyEvent* aKeyEvent,
uint32_t aCharCode,
bool aIgnoreShiftKey)
nsXBLPrototypeHandler::KeyEventMatched(
nsIDOMKeyEvent* aKeyEvent,
uint32_t aCharCode,
const IgnoreModifierState& aIgnoreModifierState)
{
if (mDetail != -1) {
// Get the keycode or charcode of the key event.
@ -626,7 +627,7 @@ nsXBLPrototypeHandler::KeyEventMatched(nsIDOMKeyEvent* aKeyEvent,
return false;
}
return ModifiersMatchMask(aKeyEvent, aIgnoreShiftKey);
return ModifiersMatchMask(aKeyEvent, aIgnoreModifierState);
}
bool
@ -645,7 +646,7 @@ nsXBLPrototypeHandler::MouseEventMatched(nsIDOMMouseEvent* aMouseEvent)
if (mMisc != 0 && (clickcount != mMisc))
return false;
return ModifiersMatchMask(aMouseEvent);
return ModifiersMatchMask(aMouseEvent, IgnoreModifierState());
}
struct keyCodeData {
@ -917,8 +918,9 @@ nsXBLPrototypeHandler::ReportKeyConflict(const char16_t* aKey, const char16_t* a
}
bool
nsXBLPrototypeHandler::ModifiersMatchMask(nsIDOMUIEvent* aEvent,
bool aIgnoreShiftKey)
nsXBLPrototypeHandler::ModifiersMatchMask(
nsIDOMUIEvent* aEvent,
const IgnoreModifierState& aIgnoreModifierState)
{
WidgetInputEvent* inputEvent = aEvent->GetInternalNSEvent()->AsInputEvent();
NS_ENSURE_TRUE(inputEvent, false);
@ -929,13 +931,13 @@ nsXBLPrototypeHandler::ModifiersMatchMask(nsIDOMUIEvent* aEvent,
}
}
if (mKeyMask & cOSMask) {
if ((mKeyMask & cOSMask) && !aIgnoreModifierState.mOS) {
if (inputEvent->IsOS() != ((mKeyMask & cOS) != 0)) {
return false;
}
}
if (mKeyMask & cShiftMask && !aIgnoreShiftKey) {
if (mKeyMask & cShiftMask && !aIgnoreModifierState.mShift) {
if (inputEvent->IsShift() != ((mKeyMask & cShift) != 0)) {
return false;
}

View File

@ -45,8 +45,30 @@ class EventTarget;
#define NS_PHASE_TARGET 2
#define NS_PHASE_BUBBLING 3
namespace mozilla {
namespace dom {
struct IgnoreModifierState
{
// When mShift is true, Shift key state will be ignored.
bool mShift;
// When mOS is true, OS key state will be ignored.
bool mOS;
IgnoreModifierState()
: mShift(false)
, mOS(false)
{
}
};
} // namespace dom
} // namespace mozilla
class nsXBLPrototypeHandler
{
typedef mozilla::dom::IgnoreModifierState IgnoreModifierState;
public:
// This constructor is used by XBL handlers (both the JS and command shorthand variety)
nsXBLPrototypeHandler(const char16_t* aEvent, const char16_t* aPhase,
@ -69,17 +91,17 @@ public:
// if aCharCode is not zero, it is used instead of the charCode of aKeyEvent.
bool KeyEventMatched(nsIDOMKeyEvent* aKeyEvent,
uint32_t aCharCode = 0,
bool aIgnoreShiftKey = false);
uint32_t aCharCode,
const IgnoreModifierState& aIgnoreModifierState);
inline bool KeyEventMatched(nsIAtom* aEventType,
nsIDOMKeyEvent* aEvent,
uint32_t aCharCode = 0,
bool aIgnoreShiftKey = false)
nsIDOMKeyEvent* aEvent,
uint32_t aCharCode,
const IgnoreModifierState& aIgnoreModifierState)
{
if (aEventType != mEventName)
return false;
return KeyEventMatched(aEvent, aCharCode, aIgnoreShiftKey);
return KeyEventMatched(aEvent, aCharCode, aIgnoreModifierState);
}
bool MouseEventMatched(nsIDOMMouseEvent* aMouseEvent);
@ -163,7 +185,7 @@ protected:
void ReportKeyConflict(const char16_t* aKey, const char16_t* aModifiers, nsIContent* aElement, const char *aMessageName);
void GetEventType(nsAString& type);
bool ModifiersMatchMask(nsIDOMUIEvent* aEvent,
bool aIgnoreShiftKey = false);
const IgnoreModifierState& aIgnoreModifierState);
nsresult DispatchXBLCommand(mozilla::dom::EventTarget* aTarget, nsIDOMEvent* aEvent);
nsresult DispatchXULKeyCommand(nsIDOMEvent* aEvent);
nsresult EnsureEventHandler(mozilla::dom::AutoJSAPI& jsapi, nsIAtom* aName,

View File

@ -357,13 +357,15 @@ nsXBLWindowKeyHandler::HandleEventOnCapture(nsIDOMKeyEvent* aEvent)
// See if the given handler cares about this particular key event
//
bool
nsXBLWindowKeyHandler::EventMatched(nsXBLPrototypeHandler* inHandler,
nsIAtom* inEventType,
nsIDOMKeyEvent* inEvent,
uint32_t aCharCode, bool aIgnoreShiftKey)
nsXBLWindowKeyHandler::EventMatched(
nsXBLPrototypeHandler* aHandler,
nsIAtom* aEventType,
nsIDOMKeyEvent* aEvent,
uint32_t aCharCode,
const IgnoreModifierState& aIgnoreModifierState)
{
return inHandler->KeyEventMatched(inEventType, inEvent, aCharCode,
aIgnoreShiftKey);
return aHandler->KeyEventMatched(aEventType, aEvent, aCharCode,
aIgnoreModifierState);
}
bool
@ -437,25 +439,29 @@ nsXBLWindowKeyHandler::WalkHandlersInternal(nsIDOMKeyEvent* aKeyEvent,
if (accessKeys.IsEmpty()) {
return WalkHandlersAndExecute(aKeyEvent, aEventType, aHandler,
0, false, aExecute);
0, IgnoreModifierState(), aExecute);
}
for (uint32_t i = 0; i < accessKeys.Length(); ++i) {
nsShortcutCandidate &key = accessKeys[i];
IgnoreModifierState ignoreModifierState;
ignoreModifierState.mShift = key.mIgnoreShift;
if (WalkHandlersAndExecute(aKeyEvent, aEventType, aHandler,
key.mCharCode, key.mIgnoreShift, aExecute))
key.mCharCode, ignoreModifierState, aExecute)) {
return true;
}
}
return false;
}
bool
nsXBLWindowKeyHandler::WalkHandlersAndExecute(nsIDOMKeyEvent* aKeyEvent,
nsIAtom* aEventType,
nsXBLPrototypeHandler* aHandler,
uint32_t aCharCode,
bool aIgnoreShiftKey,
bool aExecute)
nsXBLWindowKeyHandler::WalkHandlersAndExecute(
nsIDOMKeyEvent* aKeyEvent,
nsIAtom* aEventType,
nsXBLPrototypeHandler* aHandler,
uint32_t aCharCode,
const IgnoreModifierState& aIgnoreModifierState,
bool aExecute)
{
nsresult rv;
@ -469,8 +475,9 @@ nsXBLWindowKeyHandler::WalkHandlersAndExecute(nsIDOMKeyEvent* aKeyEvent,
}
if (!EventMatched(currHandler, aEventType, aKeyEvent,
aCharCode, aIgnoreShiftKey))
aCharCode, aIgnoreModifierState)) {
continue; // try the next one
}
// Before executing this handler, check that it's not disabled,
// and that it has something to do (oncommand of the <key> or its
@ -535,6 +542,23 @@ nsXBLWindowKeyHandler::WalkHandlersAndExecute(nsIDOMKeyEvent* aKeyEvent,
}
}
#ifdef XP_WIN
// Windows native applications ignore Windows-Logo key state when checking
// shortcut keys even if the key is pressed. Therefore, if there is no
// shortcut key which exactly matches current modifier state, we should
// retry to look for a shortcut key without the Windows-Logo key press.
if (!aIgnoreModifierState.mOS) {
WidgetKeyboardEvent* keyEvent =
aKeyEvent->GetInternalNSEvent()->AsKeyboardEvent();
if (keyEvent && keyEvent->IsOS()) {
IgnoreModifierState ignoreModifierState(aIgnoreModifierState);
ignoreModifierState.mOS = true;
return WalkHandlersAndExecute(aKeyEvent, aEventType, aHandler, aCharCode,
ignoreModifierState, aExecute);
}
}
#endif
return false;
}

View File

@ -19,11 +19,14 @@ namespace mozilla {
namespace dom {
class Element;
class EventTarget;
struct IgnoreModifierState;
}
}
class nsXBLWindowKeyHandler : public nsIDOMEventListener
{
typedef mozilla::dom::IgnoreModifierState IgnoreModifierState;
public:
nsXBLWindowKeyHandler(nsIDOMElement* aElement, mozilla::dom::EventTarget* aTarget);
@ -41,11 +44,12 @@ protected:
nsXBLPrototypeHandler* aHandler,
bool aExecute);
// walk the handlers for aEvent, aCharCode and aIgnoreShiftKey. Execute it
// if aExecute = true.
// walk the handlers for aEvent, aCharCode and aIgnoreModifierState. Execute
// it if aExecute = true.
bool WalkHandlersAndExecute(nsIDOMKeyEvent* aKeyEvent, nsIAtom* aEventType,
nsXBLPrototypeHandler* aHandler,
uint32_t aCharCode, bool aIgnoreShiftKey,
uint32_t aCharCode,
const IgnoreModifierState& aIgnoreModifierState,
bool aExecute);
// HandleEvent function for the capturing phase.
@ -59,9 +63,9 @@ protected:
nsresult EnsureHandlers();
// check if the given handler cares about the given key event
bool EventMatched(nsXBLPrototypeHandler* inHandler, nsIAtom* inEventType,
nsIDOMKeyEvent* inEvent, uint32_t aCharCode,
bool aIgnoreShiftKey);
bool EventMatched(nsXBLPrototypeHandler* aHandler, nsIAtom* aEventType,
nsIDOMKeyEvent* aEvent, uint32_t aCharCode,
const IgnoreModifierState& aIgnoreModifierState);
// Is an HTML editable element focused
bool IsHTMLEditableFieldFocused();

View File

@ -17,13 +17,17 @@ SimpleTest.waitForExplicitFinish();
var gExpected = null;
const kIsWin = navigator.platform.indexOf("Win") >= 0;
// Only on Windows, osKey state is ignored when there is no shortcut key handler
// which exactly matches with osKey state.
var keysToTest = [
["k-v", "V", { } ],
["", "V", { shiftKey: true } ],
["k-v-scy", "V", { ctrlKey: true } ],
["", "V", { altKey: true } ],
["", "V", { metaKey: true } ],
["", "V", { osKey: true } ],
[kIsWin ? "k-v" : "", "V", { osKey: true } ],
["k-v-scy", "V", { shiftKey: true, ctrlKey: true } ],
["", "V", { shiftKey: true, ctrlKey: true, altKey: true } ],
["k-e-y", "E", { } ],
@ -31,22 +35,26 @@ var keysToTest = [
["", "E", { ctrlKey: true } ],
["", "E", { altKey: true } ],
["", "E", { metaKey: true } ],
["", "E", { osKey: true } ],
[kIsWin ? "k-e-y" : "", "E", { osKey: true } ],
["k-d-a", "D", { altKey: true } ],
["k-8-m", "8", { metaKey: true } ],
["", "8", { metaKey: true, osKey: true } ],
[kIsWin ? "k-8-m" : "", "8", { metaKey: true, osKey: true } ],
["k-a-o", "A", { osKey: true } ],
["", "A", { osKey: true, metaKey: true } ],
["", "B", {} ],
["k-b-myo", "B", { osKey: true } ],
["k-b-myo", "B", { osKey: true, metaKey: true } ],
["k-f-oym", "F", { metaKey: true } ],
["k-f-oym", "F", { metaKey: true, osKey: true } ],
["k-c-scaym", "C", { metaKey: true } ],
["k-c-scaym", "C", { shiftKey: true, ctrlKey: true, altKey: true, metaKey: true } ],
[kIsWin ? "k-c-scaym" : "", "C", { shiftKey: true, ctrlKey: true, altKey: true, metaKey: true, osKey: true } ],
["", "V", { shiftKey: true, ctrlKey: true, altKey: true } ],
["k-h-l", "H", { accelKey: true } ],
// ["k-j-s", "J", { accessKey: true } ],
["", "T", { } ],
["k-g-c", "G", { ctrlKey: true } ],
["k-g-co", "G", { ctrlKey: true, osKey: true } ],
["scommand", "Y", { } ],
["", "U", { } ],
];
@ -76,13 +84,13 @@ function runTest()
// now check if a menu updates its accelerator text when a key attribute is changed
var menuitem1 = $("menuitem1");
ok(accelText(menuitem1).indexOf("d") >= 0, "menuitem1 accelText before");
if (navigator.platform.indexOf("Win") != -1) {
if (kIsWin) {
ok(accelText(menuitem1).indexOf("alt") >= 0, "menuitem1 accelText modifier before");
}
menuitem1.setAttribute("key", "k-s-c");
ok(accelText(menuitem1).indexOf("s") >= 0, "menuitem1 accelText after");
if (navigator.platform.indexOf("Win") != -1) {
if (kIsWin) {
ok(accelText(menuitem1).indexOf("ctrl") >= 0, "menuitem1 accelText modifier after");
}
@ -90,7 +98,7 @@ function runTest()
is(accelText(menuitem1), "custom", "menuitem1 accelText set custom");
menuitem1.removeAttribute("acceltext");
ok(accelText(menuitem1).indexOf("s") >= 0, "menuitem1 accelText remove");
if (navigator.platform.indexOf("Win") != -1) {
if (kIsWin) {
ok(accelText(menuitem1).indexOf("ctrl") >= 0, "menuitem1 accelText modifier remove");
}
@ -98,13 +106,13 @@ function runTest()
is(accelText(menuitem2), "", "menuitem2 accelText before");
menuitem2.setAttribute("key", "k-s-c");
ok(accelText(menuitem2).indexOf("s") >= 0, "menuitem2 accelText before");
if (navigator.platform.indexOf("Win") != -1) {
if (kIsWin) {
ok(accelText(menuitem2).indexOf("ctrl") >= 0, "menuitem2 accelText modifier before");
}
menuitem2.setAttribute("key", "k-h-l");
ok(accelText(menuitem2).indexOf("h") >= 0, "menuitem2 accelText after");
if (navigator.platform.indexOf("Win") != -1) {
if (kIsWin) {
ok(accelText(menuitem2).indexOf("ctrl") >= 0, "menuitem2 accelText modifier after");
}
@ -164,6 +172,8 @@ SimpleTest.waitForFocus(runTest);
<key id="k-h-l" key="h" modifiers="accel" oncommand="checkKey(event)"/>
<key id="k-j-s" key="j" modifiers="access" oncommand="checkKey(event)"/>
<key id="k-t-y" disabled="true" key="t" oncommand="checkKey(event)"/>
<key id="k-g-c" key="g" modifiers="control" oncommand="checkKey(event)"/>
<key id="k-g-co" key="g" modifiers="control os" oncommand="checkKey(event)"/>
<key id="k-y" key="y" command="scommand"/>
<key id="k-u" key="u" command="scommand-disabled"/>
</keyset>