Hardware keyboard spacebar scrolls too far on iOS
Created attachment 265445 [details] Patch
This should be testable with the new stuff. Please make a test.
(In reply to comment #2) > This should be testable with the new stuff. Please make a test. Yup, Tim's working on one.
Created attachment 265457 [details] Patch
Comment on attachment 265457 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=265457&action=review > Source/WebKit2/ChangeLog:43 > + If we're in the middle of resending an event we didn't handle the first > + time, just ignore it. Why does this happen? Would be nice to have an explanation in a comment. > Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:89 > + [_uiEvent release]; Doesn't RetainPtr take care of this for you? > Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:2808 > + else Would prefer a return instead of an else. > Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:2815 > + if (event == _uiEventBeingResent) > + return; Rather confusing. > LayoutTests/fast/events/ios/keyboard-scrolling-distance.html:10 > + // Need to tap to focus so that key events go through (seems wrong!). Please file a bug on this.
<rdar://problem/23500681>
Comment on attachment 265457 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=265457&action=review >> Source/WebKit2/ChangeLog:43 >> + time, just ignore it. > > Why does this happen? Would be nice to have an explanation in a comment. See below. >> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:89 >> + [_uiEvent release]; > > Doesn't RetainPtr take care of this for you? It's just a property, no RetainPtr. We could, but that would be wordier. >> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:2808 >> + else > > Would prefer a return instead of an else. Seems reasonable. >> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:2815 >> + return; > > Rather confusing. Which part? We want to ignore it if we've already had a crack at the event and didn't want to handle it, and instead let it bubble further up the chain. This is very similar to what happens on Mac. On Mac, there is a comment near the member: // We keep here the event when resending it to // the application to distinguish the case of a new event from one // that has been already sent to WebCore. Do you want something like that here? Or maybe like the one in performKeyEquivalent: // WebCore has already seen the event, no need for custom processing. >> LayoutTests/fast/events/ios/keyboard-scrolling-distance.html:10 >> + // Need to tap to focus so that key events go through (seems wrong!). > > Please file a bug on this. Will do.
http://trac.webkit.org/changeset/192432
(In reply to comment #7) > >> LayoutTests/fast/events/ios/keyboard-scrolling-distance.html:10 > >> + // Need to tap to focus so that key events go through (seems wrong!). > > > > Please file a bug on this. > > Will do. Filed https://bugs.webkit.org/show_bug.cgi?id=151264