Summary: | [iOS] Should not scroll when checkbox, radio, submit, reset, or button is spacebar activated | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Daniel Bates <dbates> | ||||||||||||
Component: | Scrolling | Assignee: | Daniel Bates <dbates> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | ews-watchlist, simon.fraser, thorton, webkit-bug-importer, wenson_hsieh | ||||||||||||
Priority: | P2 | Keywords: | InRadar, PlatformOnly | ||||||||||||
Version: | WebKit Local Build | ||||||||||||||
Hardware: | iPhone / iPad | ||||||||||||||
OS: | iOS 12 | ||||||||||||||
Attachments: |
|
Description
Daniel Bates
2019-03-04 09:31:15 PST
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> |