Keyboard event synthesis tests need to return "keyboard layout not available"

RESOLVED FIXED

Status

()

Core
Widget
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: roc, Assigned: roc)

Tracking

Trunk
x86
Windows XP
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

On Windows, intl keyboard layouts needed by keyboard event tests need to be manually installed/enabled, otherwise tests will fail. Bug 432362 requests that these layouts be enabled on tinderbox unit test machines. However, people running mochitests at home may not have these layouts enabled and we don't want their tests to fail nor do we want to make installing these layouts a barrier to running mochitests.

So I want to allow sendNativeKeyEvent to return a special nsresult to indicate that the keyboard layout could not be installed. Our key event tests can detect that and treat the test failures as expected failures.
Created attachment 319631 [details] [diff] [review]
fix

This implements what I mentioned in comment #0.

Actually, once I started using LoadKeyboardLayout here, it turns out that we can use any keyboard layout that's on the system even if it hasn't been manually enabled. But this return value could still be useful so I think we should have it.
Attachment #319631 - Flags: review?(mozbugz)
Comment on attachment 319631 [details] [diff] [review]
fix

+     * @return NS_ERROR_UNEXPECTED to indicate that the keyboard
+     * layout is not supported and the event was not fired
      */

Failure is not really unexpected if the layout is not installed (e.g. East
Asian layouts) or it's not implemented on the platform (GTK).  What about
NS_ERROR_FAILURE (and _NOT_IMPLEMENTED when appropriate)?

-    virtual void SynthesizeNativeKeyEvent(PRInt32 aNativeKeyboardLayout,
+    virtual nsresult SynthesizeNativeKeyEvent(PRInt32 aNativeKeyboardLayout,
                                           PRInt32 aNativeKeyCode,
                                           PRUint32 aModifierFlags,
                                           const nsAString& aCharacters,
                                           const nsAString& aUnmodifiedCharacters) = 0;
^ alignment.
NS_ERROR_FAILURE sounds too generic to me. How about using NS_ERROR_NOT_AVAILABLE?

http://mxr.mozilla.org/seamonkey/source/xpcom/base/nsError.h
Comment on attachment 319631 [details] [diff] [review]
fix

(In reply to comment #4)
> NS_ERROR_FAILURE sounds too generic to me. How about using
> NS_ERROR_NOT_AVAILABLE?

Ah yes.  Didn't see that.  Sounds great r=me with that.
Attachment #319631 - Flags: review?(mozbugz) → review+
Created attachment 319661 [details] [diff] [review]
fix v2

This change only affects tests. It improves our ability to test foreign keyboard layouts on Windows.
Attachment #319631 - Attachment is obsolete: true
Attachment #319661 - Flags: review+
Attachment #319661 - Flags: approval1.9?
Comment on attachment 319661 [details] [diff] [review]
fix v2

a1.9=beltzner
Attachment #319661 - Flags: approval1.9? → approval1.9+
checked in
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.