Bug 211472 - [iOS] ASSERTION FAILED: !(_keyboardFlags & WebEventKeyboardInputModifierFlagsChanged) in -[WebEvent charactersIgnoringModifiers] when pressing modifier on PDF
Summary: [iOS] ASSERTION FAILED: !(_keyboardFlags & WebEventKeyboardInputModifierFlags...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Local Build
Hardware: iPhone / iPad iOS 13
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar, PlatformOnly
Depends on:
Blocks:
 
Reported: 2020-05-05 14:38 PDT by Daniel Bates
Modified: 2020-05-06 11:33 PDT (History)
2 users (show)

See Also:


Attachments
Patch (1.88 KB, patch)
2020-05-05 15:31 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch and test (3.80 KB, patch)
2020-05-05 17:19 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2020-05-05 14:38:50 PDT
Steps to reproduce:

Using a hardware keyboard.

1. Load a PDF
2. Press the Command key on the keyboard.

Then assert !(_keyboardFlags & WebEventKeyboardInputModifierFlagsChanged) fails in -[WebEvent charactersIgnoringModifiers]:

[[
#1  0x000000011ab0c3b0 in WTFCrashWithInfo(int, char const*, char const*, int) at /Volumes/.../Source/WTF/wtf/Assertions.h:671
#2  0x000000011e1f8e58 in -[WebEvent charactersIgnoringModifiers] at /Volumes/.../Source/WebCore/platform/ios/WebEvent.mm:389
#3  0x000000010921c908 in -[WKKeyboardScrollingAnimator keyboardScrollForEvent:] at /Volumes/.../Source/WebKit/UIProcess/ios/WKKeyboardScrollingAnimator.mm:187
#4  0x000000010921d320 in -[WKKeyboardScrollingAnimator beginWithEvent:] at /Volumes/.../Source/WebKit/UIProcess/ios/WKKeyboardScrollingAnimator.mm:314
#5  0x000000010921eca8 in -[WKKeyboardScrollViewAnimator beginWithEvent:] at /Volumes/.../Source/WebKit/UIProcess/ios/WKKeyboardScrollingAnimator.mm:546
#6  0x0000000109222e5c in -[WKPDFView web_handleKeyEvent:] at /Volumes/.../Source/WebKit/UIProcess/ios/WKPDFView.mm:96
#7  0x0000000108d97a0c in -[WKWebView(WKViewInternalIOS) _handleKeyUIEvent:] at /Volumes/.../Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm:443
...
]]
Comment 1 Daniel Bates 2020-05-05 15:31:42 PDT
Created attachment 398561 [details]
Patch
Comment 2 Daniel Bates 2020-05-05 15:32:16 PDT
Tried to write a test, but it timesout:

[[
<!DOCTYPE html><!-- webkit-test-runner [ enableBackForwardCache=true ] -->
<html>
<head>
<script>
if (!window.testRunner)
    document.write("Must run in WebKitTestRunner");
else {
    testRunner.dumpAsText();
    testRunner.queueLoad("resources/test.pdf");
    testRunner.queueLoadingScript('testRunner.runUIScript(`uiController.keyDown("", ["leftCommand"])`, () => { testRunner.queueBackNavigation(1); })');
} 
</script>
</head>
<body>PASS</body>
</html>
]]
Comment 3 Daniel Bates 2020-05-05 15:52:22 PDT
(In reply to Daniel Bates from comment #2)
> Tried to write a test, but it timesout:
> 
> [[
> <!DOCTYPE html><!-- webkit-test-runner [ enableBackForwardCache=true ] -->
> <html>
> <head>
> <script>
> if (!window.testRunner)
>     document.write("Must run in WebKitTestRunner");
> else {
>     testRunner.dumpAsText();
>     testRunner.queueLoad("resources/test.pdf");
>    
> testRunner.queueLoadingScript('testRunner.runUIScript(`uiController.
> keyDown("", ["leftCommand"])`, () => { testRunner.queueBackNavigation(1);
> })');
> } 
> </script>
> </head>
> <body>PASS</body>
> </html>
> ]]

^^^ Test triggers to assertion failure without the patch, but times out with the patch.
Comment 4 Daniel Bates 2020-05-05 15:54:57 PDT
(In reply to Daniel Bates from comment #3)
> (In reply to Daniel Bates from comment #2)
> > Tried to write a test, but it timesout:
> > 
> > [[
> > <!DOCTYPE html><!-- webkit-test-runner [ enableBackForwardCache=true ] -->
> > <html>
> > <head>
> > <script>
> > if (!window.testRunner)
> >     document.write("Must run in WebKitTestRunner");
> > else {
> >     testRunner.dumpAsText();
> >     testRunner.queueLoad("resources/test.pdf");
> >    
> > testRunner.queueLoadingScript('testRunner.runUIScript(`uiController.
> > keyDown("", ["leftCommand"])`, () => { testRunner.queueBackNavigation(1);
> > })');
> > } 
> > </script>
> > </head>
> > <body>PASS</body>
> > </html>
> > ]]
> 
> ^^^ Test triggers to assertion failure without the patch, but times out with
> the patch.

My guess, timeout because testRunner.queueLoadingScript() just eval()s the string, but that string, registers a callback, which is never run. I need to wait until the UI process has processed the script though...
Comment 5 Darin Adler 2020-05-05 17:18:52 PDT
Comment on attachment 398561 [details]
Patch

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

> Source/WebKit/ChangeLog:12
> +        I tried to write a test for this using testRunner.queueLoad() and queueLoadingScript(),
> +        but I couldn't get it to not timeout. For some reason script execution doesn't seem to
> +        work after loading the PDF.

Hope you haven’t given up entirely.
Comment 6 Daniel Bates 2020-05-05 17:19:32 PDT
Created attachment 398571 [details]
Patch and test

Now with a test.
Comment 7 Daniel Bates 2020-05-06 11:32:14 PDT
Thanks for the review, Darin.
Comment 8 Daniel Bates 2020-05-06 11:32:51 PDT
Comment on attachment 398571 [details]
Patch and test

Clearing flags on attachment: 398571

Committed r261240: <https://trac.webkit.org/changeset/261240>
Comment 9 Daniel Bates 2020-05-06 11:32:53 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2020-05-06 11:33:15 PDT
<rdar://problem/62938942>