Bug 163506 – REGRESSION (r206750): Crash when pressing Caps Lock if “Use the Caps Lock key to switch to and from U.S.” is selected in Input Sources preferences

• Status: RESOLVED
• Resolution: FIXED
• Component: UI Events

<rdar://problem/28792483>

To reproduce:
1. In System Preferences > Keyboard > Input Sources, select “Use the Caps Lock key to switch to and from U.S.”
2. In Safari or in MiniBrowser, focus the webpage and press the Caps Lock key

Result:
An Objective-C exception (Invalid message sent to event "NSEvent: type=FlagsChanged loc=(-307.102,1057.25) time=1336829.1 flags=0x100 win=0x6080001e0900 winNum=34673 ctxt=0x0 keyCode=255") is raised:

#0	0x00007fffcbd1dc7d in objc_exception_throw ()
#1	0x00007fffb764a232 in +[NSException raise:format:arguments:] ()
#2	0x00007fffb907d390 in -[NSAssertionHandler handleFailureInMethod:object:file:lineNumber:description:] ()
#3	0x00007fffb53e678c in -[NSEvent characters] ()
#4	0x0000000108f8480a in WebCore::keyForKeyEvent(NSEvent*) at Source/WebCore/platform/mac/PlatformEventFactoryMac.mm:277
#5	0x0000000100730fac in WebKit::WebEventFactory::createWebKeyboardEvent(NSEvent*, bool, bool, WTF::Vector<WebCore::KeypressCommand, 0ul, WTF::CrashOnOverflow, 16ul> const&) at Source/WebKit2/Shared/mac/WebEventFactory.mm:481
#6	0x00000001002b0458 in WebKit::NativeWebKeyboardEvent::NativeWebKeyboardEvent(NSEvent*, bool, bool, WTF::Vector<WebCore::KeypressCommand, 0ul, WTF::CrashOnOverflow, 16ul> const&) at Source/WebKit2/Shared/mac/NativeWebKeyboardEventMac.mm:39
#7	0x00000001002b070b in WebKit::NativeWebKeyboardEvent::NativeWebKeyboardEvent(NSEvent*, bool, bool, WTF::Vector<WebCore::KeypressCommand, 0ul, WTF::CrashOnOverflow, 16ul> const&) at Source/WebKit2/Shared/mac/NativeWebKeyboardEventMac.mm:41
#8	0x0000000100bbd1ca in ::___ZN6WebKit11WebViewImpl12flagsChangedEP7NSEvent_block_invoke(BOOL, const WTF::Vector<WebCore::KeypressCommand, 0, WTF::CrashOnOverflow, 16> &) at Source/WebKit2/UIProcess/Cocoa/WebViewImpl.mm:3945
#9	0x0000000100bba080 in WebKit::WebViewImpl::interpretKeyEvent(NSEvent*, void (signed char, WTF::Vector<WebCore::KeypressCommand, 0ul, WTF::CrashOnOverflow, 16ul> const&) block_pointer) at Source/WebKit2/UIProcess/Cocoa/WebViewImpl.mm:3512
#10	0x0000000100bbd0f4 in WebKit::WebViewImpl::flagsChanged(NSEvent*) at Source/WebKit2/UIProcess/Cocoa/WebViewImpl.mm:3944
#11	0x0000000100ca8016 in ::-[WKWebView flagsChanged:](NSEvent *) at Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:2699

In WebKit1 MiniBrowser windows, this is preceded by an assertion failure:

#0	0x0000000104d14e64 in ::WTFCrash() at /Projects/Safari/OpenSource/Source/WTF/wtf/Assertions.cpp:323
#1	0x0000000108f85ff6 in WebCore::keyIdentifierForKeyEvent(NSEvent*) at /Projects/Safari/OpenSource/Source/WebCore/platform/mac/PlatformEventFactoryMac.mm:529
#2	0x0000000108f87b6c in WebCore::PlatformKeyboardEventBuilder::PlatformKeyboardEventBuilder(NSEvent*) at /Projects/Safari/OpenSource/Source/WebCore/platform/mac/PlatformEventFactoryMac.mm:839
#3	0x0000000108f8666d in WebCore::PlatformKeyboardEventBuilder::PlatformKeyboardEventBuilder(NSEvent*) at /Projects/Safari/OpenSource/Source/WebCore/platform/mac/PlatformEventFactoryMac.mm:830
#4	0x0000000108f86600 in WebCore::PlatformEventFactory::createPlatformKeyboardEvent(NSEvent*) at /Projects/Safari/OpenSource/Source/WebCore/platform/mac/PlatformEventFactoryMac.mm:875
#5	0x0000000103228174 in ::-[WebHTMLView flagsChanged:](NSEvent *) at /Projects/Safari/OpenSource/Source/WebKit/mac/WebView/WebHTMLView.mm:5300

Using nightly builds, I tracked the regression down to the r206737-r206763 range, and the only relevant change in that range is <https://trac.webkit.org/r206750>.

[Chris Dumez]

Thanks I will take a look tomorrow.

[Chris Dumez]

According to https://developer.apple.com/reference/appkit/nsevent/1534183-characters?language=objc :
""
This property is only valid for key-up and key-down events. It raises an NSInternalInconsistencyException if accessed on any other kind of event object.
""

The issue is that I cam calling characters to other events that keyUp and keyDown it seems.

[Chris Dumez]

Created attachment 291782 [details]
Patch

[Chris Dumez]

Created attachment 291783 [details]
Patch

Comment on attachment 291783 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=291783&action=review

> Source/WebCore/platform/mac/PlatformEventFactoryMac.mm:275
> +#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101200
> +    if ([event type] == NSEventTypeFlagsChanged)
> +#else
> +    if ([event type] == NSFlagsChanged)
> +#endif
> +        return ASCIILiteral("Unidentified");

Instead of doing this, just #import <wtf/AppKitCompatibilityDeclarations.h> so you can use the modern enum unconditionally.

[Chris Dumez]

(In reply to comment #6)
> Comment on attachment 291783 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=291783&action=review
> 
> > Source/WebCore/platform/mac/PlatformEventFactoryMac.mm:275
> > +#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101200
> > +    if ([event type] == NSEventTypeFlagsChanged)
> > +#else
> > +    if ([event type] == NSFlagsChanged)
> > +#endif
> > +        return ASCIILiteral("Unidentified");
> 
> Instead of doing this, just #import <wtf/AppKitCompatibilityDeclarations.h>
> so you can use the modern enum unconditionally.

Nice, I'll update.

[Chris Dumez]

Created attachment 291786 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 291786 [details]
Patch

Clearing flags on attachment: 291786

Committed r207397: <http://trac.webkit.org/changeset/207397>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.