Bug 163506

Summary: 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
Product: WebKit Reporter: mitz
Component: UI EventsAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Blocker CC: cdumez, commit-queue, webkit-bug-importer, webkit
Priority: P1 Keywords: InRadar, Regression
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: macOS 10.12   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

mitz
Reported 2016-10-16 11:51:33 PDT
<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
Attachments
Patch (2.79 KB, patch)
2016-10-16 16:38 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (2.90 KB, patch)
2016-10-16 16:43 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (5.81 KB, patch)
2016-10-16 17:13 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
mitz
Comment 1 2016-10-16 12:03:04 PDT
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
Comment 2 2016-10-16 12:13:39 PDT
Thanks I will take a look tomorrow.
Chris Dumez
Comment 3 2016-10-16 16:09:46 PDT
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
Comment 4 2016-10-16 16:38:49 PDT
Created attachment 291782 [details] Patch
Chris Dumez
Comment 5 2016-10-16 16:43:14 PDT
Created attachment 291783 [details] Patch
mitz
Comment 6 2016-10-16 16:45:26 PDT
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
Comment 7 2016-10-16 17:09:10 PDT
(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
Comment 8 2016-10-16 17:13:01 PDT
Created attachment 291786 [details] Patch
WebKit Commit Bot
Comment 9 2016-10-16 18:31:32 PDT
Comment on attachment 291786 [details] Patch Clearing flags on attachment: 291786 Committed r207397: <http://trac.webkit.org/changeset/207397>
WebKit Commit Bot
Comment 10 2016-10-16 18:31:36 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.