Bug 192824

Actually, this bug effects all keys.

<rdar://problem/47100332>

Actually, this bug effects only non-modifier keys.

Created attachment 358651 [details] Patch and updated tests

Created attachment 358654 [details] Patch and layout tests

Comment on attachment 358654 [details] Patch and layout tests View in context: https://bugs.webkit.org/attachment.cgi?id=358654&action=review > Source/WebKit/ChangeLog:33 > + an intercepted UIEvents for app key commands. Nit - s/an// > LayoutTests/fast/events/ios/keypress-keys-in-non-editable-element.html:53 > +// FIXME: Test that keypress is not dispatch for "insert" once <rdar://problem/47128940> is fixed. Nit - …is not dispatched…

*** Bug 193255 has been marked as a duplicate of this bug. ***

Created attachment 358962 [details] To Land once <rdar://problem/47222115> is fixed I will land this once the fix for <rdar://problem/47222115> lands.

Comment on attachment 358962 [details] To Land once <rdar://problem/47222115> is fixed View in context: https://bugs.webkit.org/attachment.cgi?id=358962&action=review > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:3807 > + // Do not change traits when dismissing the keyboard. > + if (_isBlurringFocusedNode) > + return _traits.get(); For completeness, this code is new in this version of the patch. We need to do this to ensure that we do not change the keyboard's appearance as part of its dismissal animation now that the keyboard sticks around (though its UI is hidden) when nothing is focused/a non-editable element is focused.

Created attachment 359345 [details] To Land once <rdar://problem/47222115> is fixed Condition return value of _disableAutomaticKeyboardUI on whether we are first responder. Otherwise, we interfere with the Find-in-page feature.

Created attachment 359978 [details] Patch and layout test Updated patch to fix tab cycling and type ahead for <select>s.

Comment on attachment 359978 [details] Patch and layout test View in context: https://bugs.webkit.org/attachment.cgi?id=359978&action=review > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:3819 > + // Do not change traits when dismissing the keyboard. > + if (_isBlurringFocusedNode) > + return _traits.get(); > + I think this is fine as-is, but I'll guard this code with USE(UIKIT_KEYBOARD_ADDITIONS) before landing since this is really only needed now that -_requiresKeyboardWhenFirstResponder always returns YES. > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:4657 > + [self _endEditing]; I think this is fine as-is, but I'll guard this code with USE(UIKIT_KEYBOARD_ADDITIONS) before landing since this is really only needed now that -_requiresKeyboardWhenFirstResponder always returns YES. > LayoutTests/ChangeLog:14 > +2019-01-23 Daniel Bates <dabates@apple.com> > + > + [iOS] Keyups for non-modifier keys identified as "Dead" when not focused in a content-editable element > + https://bugs.webkit.org/show_bug.cgi?id=192824 > + <rdar://problem/47100332> > + > + Reviewed by NOBODY (OOPS!). > + > + * fast/events/ios/keydown-keyup-special-keys-in-non-editable-element-expected.txt: > + * fast/events/ios/keydown-keyup-special-keys-in-non-editable-element.html: > + * fast/events/ios/keypress-keys-in-non-editable-element-expected.txt: > + * fast/events/ios/keypress-keys-in-non-editable-element.html: > + > +2019-01-11 Daniel Bates <dabates@apple.com> Will fix this double ChangeLog before landing.

Created attachment 360632 [details] To land

Comment on attachment 360632 [details] To land Clearing flags on attachment: 360632 Committed r240742: <https://trac.webkit.org/changeset/240742>

All reviewed patches have been landed. Closing bug.

Reverted r240742 for reason: Causes crashes on iOS simulator. Committed r240983: <https://trac.webkit.org/changeset/240983>

Created attachment 361561 [details] Patch and layout tests Patch -applyAutocorrection and -requestAutocorrectionContextWithCompletionHandler to not perform the replacement or compute autocorrection data when we are not in an editable element.

Created attachment 361590 [details] Patch and layout test

Created attachment 361632 [details] Patch and layout tests

Comment on attachment 361632 [details] Patch and layout tests View in context: https://bugs.webkit.org/attachment.cgi?id=361632&action=review > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:3400 > + if (!_page->editorState().isContentEditable) { This seems like an acceptable workaround for now, but in general we should not be relying on cached state in the UI process for correctness in asynchronous methods like this. Is there ever a case in which the editor state is not up to date when UIKit asks us for this? I imagine that in -applyAutocorrection: it's probably okay, but I suspect that when we first focus a field to summon the keyboard, -requestAutocorrectionContextWithCompletionHandler: *might* be invoked before our EditorState has arrived in the UI process.

(In reply to Wenson Hsieh from comment #22) > Comment on attachment 361632 [details] > Patch and layout tests > > View in context: > https://bugs.webkit.org/attachment.cgi?id=361632&action=review > > > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:3400 > > + if (!_page->editorState().isContentEditable) { > > This seems like an acceptable workaround for now, but in general we should > not be relying on cached state in the UI process for correctness in > asynchronous methods like this. > > Is there ever a case in which the editor state is not up to date when UIKit > asks us for this? I imagine that in -applyAutocorrection: it's probably > okay, but I suspect that when we first focus a field to summon the keyboard, > -requestAutocorrectionContextWithCompletionHandler: *might* be invoked > before our EditorState has arrived in the UI process. This if-condition was a last minute change. Originally I conditioned on [self _disableAutomaticKeyboardUI]. I just want to avoid calling this function when we are not focused in an editable element. If nothing is focused and editable OR a non-text field is focused (like a <select>) then no need to request or apply corrections, right? Maybe I should go back to [self _disableAutomaticKeyboardUI]? Or is there a better way to get what I want?

Comment on attachment 361632 [details] Patch and layout tests View in context: https://bugs.webkit.org/attachment.cgi?id=361632&action=review >>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:3400 >>> + if (!_page->editorState().isContentEditable) { >> >> This seems like an acceptable workaround for now, but in general we should not be relying on cached state in the UI process for correctness in asynchronous methods like this. >> >> Is there ever a case in which the editor state is not up to date when UIKit asks us for this? I imagine that in -applyAutocorrection: it's probably okay, but I suspect that when we first focus a field to summon the keyboard, -requestAutocorrectionContextWithCompletionHandler: *might* be invoked before our EditorState has arrived in the UI process. > > This if-condition was a last minute change. Originally I conditioned on [self _disableAutomaticKeyboardUI]. I just want to avoid calling this function when we are not focused in an editable element. If nothing is focused and editable OR a non-text field is focused (like a <select>) then no need to request or apply corrections, right? Maybe I should go back to [self _disableAutomaticKeyboardUI]? Or is there a better way to get what I want? _disableAutomaticKeyboardUI uses focused element info, which should definitely be up to date by the time UIKit calls into us for an autocorrection context. The editor state will arrive later, possibly after UIKit asks us for an autocorrection context.

(In reply to Wenson Hsieh from comment #24) > Comment on attachment 361632 [details] > Patch and layout tests > > View in context: > https://bugs.webkit.org/attachment.cgi?id=361632&action=review > > >>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:3400 > >>> + if (!_page->editorState().isContentEditable) { > >> > >> This seems like an acceptable workaround for now, but in general we should not be relying on cached state in the UI process for correctness in asynchronous methods like this. > >> > >> Is there ever a case in which the editor state is not up to date when UIKit asks us for this? I imagine that in -applyAutocorrection: it's probably okay, but I suspect that when we first focus a field to summon the keyboard, -requestAutocorrectionContextWithCompletionHandler: *might* be invoked before our EditorState has arrived in the UI process. > > > > This if-condition was a last minute change. Originally I conditioned on [self _disableAutomaticKeyboardUI]. I just want to avoid calling this function when we are not focused in an editable element. If nothing is focused and editable OR a non-text field is focused (like a <select>) then no need to request or apply corrections, right? Maybe I should go back to [self _disableAutomaticKeyboardUI]? Or is there a better way to get what I want? > > _disableAutomaticKeyboardUI uses focused element info, which should > definitely be up to date by the time UIKit calls into us for an > autocorrection context. The editor state will arrive later, possibly after > UIKit asks us for an autocorrection context. Okiedokies. Will change to use that.

Created attachment 362317 [details] To land

Created attachment 362319 [details] To land

Committed r241734: <https://trac.webkit.org/changeset/241734>