With recent UIKit work and the patch for bug #192824 checkboxes and radio buttons can now be activated by pressing the spacebar when focused. The spacebar is also interpreted by the iOS scroll animator as a request to scroll the page. So, we may scroll the page when trying to activate a checkbox or radio button. That does not feel natural. We have heuristics to not scroll when the spacebar is pressed if the currently focused element is a content editable element or a <select>. Clearly this does not cover checkboxes and radio buttons. So, we should amend the heuristic. Also, we should consider amending the heuristic to avoid scrolling when other form elements are focused such as a submit, reset or button element. Note that content edibility covers text fields (all the different kinds) and text areas.
<rdar://problem/48564347>
Created attachment 363568 [details] Patch and tests
Created attachment 363572 [details] Patch and tests
Comment on attachment 363572 [details] Patch and tests Attachment 363572 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/11370023 New failing tests: fast/events/beforeunload-alert-user-interaction2.html fast/forms/datalist/datalist-textinput-suggestions-order.html fast/images/imagemap-polygon-focus-ring.html fast/scrolling/ios/hit-testing-iframe-005.html editing/selection/thai-word-at-document-end.html fast/events/click-handler-on-body-simple.html
Created attachment 363582 [details] Archive of layout-test-results from ews125 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
(In reply to Build Bot from comment #4) > Comment on attachment 363572 [details] > Patch and tests > > Attachment 363572 [details] did not pass ios-sim-ews (ios-simulator-wk2): > Output: https://webkit-queues.webkit.org/results/11370023 > > New failing tests: > fast/events/beforeunload-alert-user-interaction2.html > fast/forms/datalist/datalist-textinput-suggestions-order.html > fast/images/imagemap-polygon-focus-ring.html > fast/scrolling/ios/hit-testing-iframe-005.html > editing/selection/thai-word-at-document-end.html > fast/events/click-handler-on-body-simple.html Just eyeballing the EWS results and these tests failed on trunk without my patch. Confirmed locally that all of these tests pass with my patch.
(In reply to Daniel Bates from comment #6) > (In reply to Build Bot from comment #4) > > Comment on attachment 363572 [details] > > Patch and tests > > > > Attachment 363572 [details] did not pass ios-sim-ews (ios-simulator-wk2): > > Output: https://webkit-queues.webkit.org/results/11370023 > > > > New failing tests: > > fast/events/beforeunload-alert-user-interaction2.html > > fast/forms/datalist/datalist-textinput-suggestions-order.html > > fast/images/imagemap-polygon-focus-ring.html > > fast/scrolling/ios/hit-testing-iframe-005.html > > editing/selection/thai-word-at-document-end.html > > fast/events/click-handler-on-body-simple.html > > Just eyeballing the EWS results and these tests failed on trunk without my > patch. Confirmed locally that all of these tests pass with my patch. Looks like this was fallout from r242403 (since reverted in r242467).
Comment on attachment 363572 [details] Patch and tests View in context: https://bugs.webkit.org/attachment.cgi?id=363572&action=review > Source/WebCore/ChangeLog:17 > + WebCore implements spacebar activation on keydown for form controls. For IE compatibility > + WebCore does not mark such keydown events as handled so that a DOM keypress event will > + be subsequently dispatched. The current logic only skips calling the base class's default Do we still care about IE combat? What do other browsers do? Maybe HTML5 specifies this now? > Source/WebCore/ChangeLog:21 > + event is not marked as handled, but WebCore actaully accounted for this event then we need actaully > Source/WebCore/html/BaseCheckableInputType.cpp:67 > -void BaseCheckableInputType::handleKeydownEvent(KeyboardEvent& event) > +auto BaseCheckableInputType::handleKeydownEvent(KeyboardEvent& event) -> ShouldCallBaseEventHandler I kinda feel like a bool return value would be as easily understood. > Source/WebCore/html/BaseChooserOnlyDateAndTimeInputType.cpp:120 > BaseClickableWithKeyInputType::handleKeydownEvent(*element(), event); > + return ShouldCallBaseEventHandler::Yes; Why not return BaseClickableWithKeyInputType::handleKeydownEvent() ?
(In reply to Simon Fraser (smfr) from comment #8) > Comment on attachment 363572 [details] > Patch and tests > > View in context: > https://bugs.webkit.org/attachment.cgi?id=363572&action=review > > > Source/WebCore/ChangeLog:17 > > + WebCore implements spacebar activation on keydown for form controls. For IE compatibility > > + WebCore does not mark such keydown events as handled so that a DOM keypress event will > > + be subsequently dispatched. The current logic only skips calling the base class's default > > Do we still care about IE combat? I don't know. > What do other browsers do? Chrome and Firefox also dispatch keypress. > Maybe HTML5 > specifies this now? > Nope. Filed <https://github.com/w3c/uievents/issues/225> to fix up UI Events spec. > > Source/WebCore/ChangeLog:21 > > + event is not marked as handled, but WebCore actaully accounted for this event then we need > > actaully > Will fix, > > Source/WebCore/html/BaseCheckableInputType.cpp:67 > > -void BaseCheckableInputType::handleKeydownEvent(KeyboardEvent& event) > > +auto BaseCheckableInputType::handleKeydownEvent(KeyboardEvent& event) -> ShouldCallBaseEventHandler > > I kinda feel like a bool return value would be as easily understood. > > > Source/WebCore/html/BaseChooserOnlyDateAndTimeInputType.cpp:120 > > BaseClickableWithKeyInputType::handleKeydownEvent(*element(), event); > > + return ShouldCallBaseEventHandler::Yes; > > Why not return BaseClickableWithKeyInputType::handleKeydownEvent() ? I could, but I need to teach that overload about ShouldCallBaseEventHandler and it would always return YES. I guess I could :/
Comment on attachment 363572 [details] Patch and tests View in context: https://bugs.webkit.org/attachment.cgi?id=363572&action=review >> Source/WebCore/html/BaseChooserOnlyDateAndTimeInputType.cpp:120 >> + return ShouldCallBaseEventHandler::Yes; > > Why not return BaseClickableWithKeyInputType::handleKeydownEvent() ? Will fix.
Created attachment 363698 [details] To land
Created attachment 363701 [details] To land
Committed r242518: <https://trac.webkit.org/changeset/242518>