Diff
Modified: branches/safari-601.1.46-branch/LayoutTests/ChangeLog (192978 => 192979)
--- branches/safari-601.1.46-branch/LayoutTests/ChangeLog 2015-12-02 23:04:28 UTC (rev 192978)
+++ branches/safari-601.1.46-branch/LayoutTests/ChangeLog 2015-12-02 23:04:34 UTC (rev 192979)
@@ -1,5 +1,22 @@
2015-12-01 Matthew Hanson <matthew_han...@apple.com>
+ Merge r192432. rdar://problem/23558672
+
+ 2015-11-13 Tim Horton <timothy_hor...@apple.com>
+
+ Hardware keyboard spacebar scrolls too far on iOS
+ https://bugs.webkit.org/show_bug.cgi?id=151227
+ <rdar://problem/23500681>
+
+ Reviewed by Simon Fraser.
+
+ * fast/events/ios/keyboard-scrolling-distance-expected.txt: Added.
+ * fast/events/ios/keyboard-scrolling-distance.html: Added.
+ Add a test that records how much we scroll when pressing the spacebar.
+ The test is at a fixed scale of 1.5 to expose the bug fixed by this patch.
+
+2015-12-01 Matthew Hanson <matthew_han...@apple.com>
+
Merge r192169. rdar://problem/23189763
2015-11-09 Myles C. Maxfield <mmaxfi...@apple.com>
Added: branches/safari-601.1.46-branch/LayoutTests/fast/events/ios/keyboard-scrolling-distance-expected.txt (0 => 192979)
--- branches/safari-601.1.46-branch/LayoutTests/fast/events/ios/keyboard-scrolling-distance-expected.txt (rev 0)
+++ branches/safari-601.1.46-branch/LayoutTests/fast/events/ios/keyboard-scrolling-distance-expected.txt 2015-12-02 23:04:34 UTC (rev 192979)
@@ -0,0 +1,6 @@
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Before pressing the spacebar, window.scrollY = 0
+After pressing the spacebar, window.scrollY = 255
+
Added: branches/safari-601.1.46-branch/LayoutTests/fast/events/ios/keyboard-scrolling-distance.html (0 => 192979)
--- branches/safari-601.1.46-branch/LayoutTests/fast/events/ios/keyboard-scrolling-distance.html (rev 0)
+++ branches/safari-601.1.46-branch/LayoutTests/fast/events/ios/keyboard-scrolling-distance.html 2015-12-02 23:04:34 UTC (rev 192979)
@@ -0,0 +1,58 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+
+<html>
+
+<head>
+ <script src=""
+ <meta name="viewport" content="initial-scale=1.5, user-scalable=no">
+ <script id="ui-script" type="text/plain">
+ (function() {
+ // FIXME (151264): Need to tap to focus so that key events go through.
+ uiController.singleTapAtPoint(50, 25, function() {
+ uiController.typeCharacterUsingHardwareKeyboard(" ", function() {
+ uiController.didEndScrollingCallback = function() {
+ uiController.uiScriptComplete();
+ };
+ });
+ });
+ })();
+ </script>
+
+ <script>
+ if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+ }
+
+ function getUIScript()
+ {
+ return document.getElementById("ui-script").text;
+ }
+
+ function runTest()
+ {
+ if (!window.testRunner || !testRunner.runUIScript)
+ return;
+
+ debug("Before pressing the spacebar, window.scrollY = " + window.scrollY);
+ testRunner.runUIScript(getUIScript(), function(result) {
+ debug("After pressing the spacebar, window.scrollY = " + window.scrollY);
+ testRunner.notifyDone();
+ });
+ }
+ </script>
+ <style>
+ #placeholder {
+ width: 100px;
+ height: 5000px;
+ }
+ </style>
+</head>
+
+<body style="margin: 0;" _onload_="runTest()">
+ <div id="placeholder">
+ <div id="console"></div>
+ <script src=""
+</body>
+
+</html>
Modified: branches/safari-601.1.46-branch/Source/WebKit2/ChangeLog (192978 => 192979)
--- branches/safari-601.1.46-branch/Source/WebKit2/ChangeLog 2015-12-02 23:04:28 UTC (rev 192978)
+++ branches/safari-601.1.46-branch/Source/WebKit2/ChangeLog 2015-12-02 23:04:34 UTC (rev 192979)
@@ -1,5 +1,64 @@
2015-12-01 Matthew Hanson <matthew_han...@apple.com>
+ Merge r192432. rdar://problem/23558672
+
+ 2015-11-13 Tim Horton <timothy_hor...@apple.com>
+
+ Hardware keyboard spacebar scrolls too far on iOS
+ https://bugs.webkit.org/show_bug.cgi?id=151227
+ <rdar://problem/23500681>
+
+ Reviewed by Simon Fraser.
+
+ There were two bugs conspiring here to make spacebar scrolling very wrong on iOS:
+
+ 1) Incoming key events were being handled twice
+ - fix this by only propagating the event to super if we don't
+ end up handling the event ourselves
+
+ 2) _scrollByOffset was not converting the offset delta out of content coordinates
+ - fix this by doing the relevant conversion and renaming the function
+ to make it more clear exactly what it does
+
+ * UIProcess/API/Cocoa/WKWebView.mm:
+ (-[WKWebView _scrollByContentOffset:]):
+ (-[WKWebView _scrollByOffset:]): Deleted.
+ * UIProcess/API/Cocoa/WKWebViewInternal.h:
+ Rename to _scrollByContentOffset to make it clear that this is
+ in content coordinates, and do the conversion from content coordinates
+ (apply the scale) so that we scroll by the right amount.
+
+ * UIProcess/ios/PageClientImplIOS.mm:
+ (WebKit::PageClientImpl::doneWithKeyEvent):
+ Pass whether the event was handled through to WKContentView.
+
+ * UIProcess/ios/WKContentViewInteraction.h:
+ * UIProcess/ios/WKContentViewInteraction.mm:
+ (-[WKWebEvent dealloc]):
+ Add WKWebEvent, which is a WebEvent and retains a reference to the UIEvent
+ from which it came.
+
+ (-[WKContentView _handleKeyUIEvent:]):
+ Only pass the event to super immediately if we don't try to handle it.
+ We'll re-send it to super asynchronously if we decide not to handle it.
+
+ (-[WKContentView handleKeyEvent:]):
+ If we're in the middle of resending an event we didn't handle the first
+ time, just ignore it.
+
+ Make a WKWebEvent and store the UIEvent on it.
+
+ (-[WKContentView _didHandleKeyEvent:eventWasHandled:]):
+ If the event wasn't handled, and is one of our special events with a
+ wrapped UIEvent, resend it to super. We'll ignore it as noted above.
+
+ (-[WKContentView _interpretKeyEvent:isCharEvent:]):
+ Factor out unobscured content rect retrieval.
+ Make use of Scrollbar::pageStep to match the Mac behavior of not
+ scrolling quite a whole page when scrolling with the spacebar.
+
+2015-12-01 Matthew Hanson <matthew_han...@apple.com>
+
Merge r187544. rdar://problem/23395970
2015-07-29 Chris Dumez <cdu...@apple.com>
Modified: branches/safari-601.1.46-branch/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm (192978 => 192979)
--- branches/safari-601.1.46-branch/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm 2015-12-02 23:04:28 UTC (rev 192978)
+++ branches/safari-601.1.46-branch/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm 2015-12-02 23:04:34 UTC (rev 192979)
@@ -1254,12 +1254,15 @@
return true;
}
-- (void)_scrollByOffset:(WebCore::FloatPoint)offset
+- (void)_scrollByContentOffset:(WebCore::FloatPoint)contentOffsetDelta
{
- CGPoint currentOffset = ([_scrollView _isAnimatingScroll]) ? [_scrollView _animatedTargetOffset] : [_scrollView contentOffset];
+ WebCore::FloatPoint scaledOffsetDelta = contentOffsetDelta;
+ CGFloat zoomScale = contentZoomScale(self);
+ scaledOffsetDelta.scale(zoomScale, zoomScale);
- CGPoint boundedOffset = contentOffsetBoundedInValidRange(_scrollView.get(), currentOffset + offset);
-
+ CGPoint currentOffset = [_scrollView _isAnimatingScroll] ? [_scrollView _animatedTargetOffset] : [_scrollView contentOffset];
+ CGPoint boundedOffset = contentOffsetBoundedInValidRange(_scrollView.get(), currentOffset + scaledOffsetDelta);
+
if (CGPointEqualToPoint(boundedOffset, currentOffset))
return;
[_contentView willStartZoomOrScroll];
Modified: branches/safari-601.1.46-branch/Source/WebKit2/UIProcess/API/Cocoa/WKWebViewInternal.h (192978 => 192979)
--- branches/safari-601.1.46-branch/Source/WebKit2/UIProcess/API/Cocoa/WKWebViewInternal.h 2015-12-02 23:04:28 UTC (rev 192978)
+++ branches/safari-601.1.46-branch/Source/WebKit2/UIProcess/API/Cocoa/WKWebViewInternal.h 2015-12-02 23:04:34 UTC (rev 192979)
@@ -83,7 +83,7 @@
- (void)_scrollToContentOffset:(WebCore::FloatPoint)contentOffset scrollOrigin:(WebCore::IntPoint)scrollOrigin;
- (BOOL)_scrollToRect:(WebCore::FloatRect)targetRect origin:(WebCore::FloatPoint)origin minimumScrollDistance:(float)minimumScrollDistance;
-- (void)_scrollByOffset:(WebCore::FloatPoint)offset;
+- (void)_scrollByContentOffset:(WebCore::FloatPoint)offset;
- (void)_zoomToFocusRect:(WebCore::FloatRect)focusedElementRect selectionRect:(WebCore::FloatRect)selectionRectInDocumentCoordinates fontSize:(float)fontSize minimumScale:(double)minimumScale maximumScale:(double)maximumScale allowScaling:(BOOL)allowScaling forceScroll:(BOOL)forceScroll;
- (BOOL)_zoomToRect:(WebCore::FloatRect)targetRect withOrigin:(WebCore::FloatPoint)origin fitEntireRect:(BOOL)fitEntireRect minimumScale:(double)minimumScale maximumScale:(double)maximumScale minimumScrollDistance:(float)minimumScrollDistance;
- (void)_zoomOutWithOrigin:(WebCore::FloatPoint)origin animated:(BOOL)animated;
Modified: branches/safari-601.1.46-branch/Source/WebKit2/UIProcess/ios/PageClientImplIOS.mm (192978 => 192979)
--- branches/safari-601.1.46-branch/Source/WebKit2/UIProcess/ios/PageClientImplIOS.mm 2015-12-02 23:04:28 UTC (rev 192978)
+++ branches/safari-601.1.46-branch/Source/WebKit2/UIProcess/ios/PageClientImplIOS.mm 2015-12-02 23:04:34 UTC (rev 192979)
@@ -429,9 +429,9 @@
return enclosingIntRect(rootViewRect);
}
-void PageClientImpl::doneWithKeyEvent(const NativeWebKeyboardEvent& event, bool)
+void PageClientImpl::doneWithKeyEvent(const NativeWebKeyboardEvent& event, bool eventWasHandled)
{
- [m_contentView _didHandleKeyEvent:event.nativeEvent()];
+ [m_contentView _didHandleKeyEvent:event.nativeEvent() eventWasHandled:eventWasHandled];
}
#if ENABLE(TOUCH_EVENTS)
Modified: branches/safari-601.1.46-branch/Source/WebKit2/UIProcess/ios/WKContentViewInteraction.h (192978 => 192979)
--- branches/safari-601.1.46-branch/Source/WebKit2/UIProcess/ios/WKContentViewInteraction.h 2015-12-02 23:04:28 UTC (rev 192978)
+++ branches/safari-601.1.46-branch/Source/WebKit2/UIProcess/ios/WKContentViewInteraction.h 2015-12-02 23:04:34 UTC (rev 192979)
@@ -145,6 +145,7 @@
WebKit::InteractionInformationAtPosition _positionInformation;
WebKit::AssistedNodeInformation _assistedNodeInformation;
RetainPtr<NSObject<WKFormPeripheral>> _inputPeripheral;
+ RetainPtr<UIEvent> _uiEventBeingResent;
CGPoint _lastInteractionLocation;
@@ -202,7 +203,7 @@
- (void)_showPlaybackTargetPicker:(BOOL)hasVideo fromRect:(const WebCore::IntRect&)elementRect;
- (void)_showRunOpenPanel:(WebKit::WebOpenPanelParameters*)parameters resultListener:(WebKit::WebOpenPanelResultListenerProxy*)listener;
- (void)accessoryDone;
-- (void)_didHandleKeyEvent:(WebIOSEvent *)event;
+- (void)_didHandleKeyEvent:(WebIOSEvent *)event eventWasHandled:(BOOL)eventWasHandled;
- (Vector<WebKit::OptionItem>&) assistedNodeSelectOptions;
- (void)_enableInspectorNodeSearch;
- (void)_disableInspectorNodeSearch;
Modified: branches/safari-601.1.46-branch/Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm (192978 => 192979)
--- branches/safari-601.1.46-branch/Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm 2015-12-02 23:04:28 UTC (rev 192978)
+++ branches/safari-601.1.46-branch/Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm 2015-12-02 23:04:34 UTC (rev 192979)
@@ -80,6 +80,20 @@
@property (nonatomic, assign) UIKeyboardInputFlags _inputFlags;
@end
+@interface WKWebEvent : WebEvent
+@property (nonatomic, retain) UIEvent *uiEvent;
+@end
+
+@implementation WKWebEvent
+
+- (void)dealloc
+{
+ [_uiEvent release];
+ [super dealloc];
+}
+
+@end
+
using namespace WebCore;
using namespace WebKit;
@@ -2625,15 +2639,21 @@
{
// We only want to handle key event from the hardware keyboard when we are
// first responder and we are not interacting with editable content.
- if ([self isFirstResponder] && event._hidEvent && !_page->editorState().isContentEditable)
+ if ([self isFirstResponder] && event._hidEvent && !_page->editorState().isContentEditable) {
[self handleKeyEvent:event];
+ return;
+ }
[super _handleKeyUIEvent:event];
}
- (void)handleKeyEvent:(::UIEvent *)event
{
- ::WebEvent *webEvent = [[[::WebEvent alloc] initWithKeyEventType:(event._isKeyDown) ? WebEventKeyDown : WebEventKeyUp
+ // WebCore has already seen the event, no need for custom processing.
+ if (event == _uiEventBeingResent)
+ return;
+
+ WKWebEvent *webEvent = [[[WKWebEvent alloc] initWithKeyEventType:(event._isKeyDown) ? WebEventKeyDown : WebEventKeyUp
timeStamp:event.timestamp
characters:event._modifiedInput
charactersIgnoringModifiers:event._unmodifiedInput
@@ -2643,6 +2663,7 @@
keyCode:0
isTabKey:[event._modifiedInput isEqualToString:@"\t"]
characterSet:WebEventCharacterSetUnicode] autorelease];
+ webEvent.uiEvent = event;
[self handleKeyWebEvent:webEvent];
}
@@ -2652,7 +2673,7 @@
_page->handleKeyboardEvent(NativeWebKeyboardEvent(theEvent));
}
-- (void)_didHandleKeyEvent:(WebIOSEvent *)event
+- (void)_didHandleKeyEvent:(WebIOSEvent *)event eventWasHandled:(BOOL)eventWasHandled
{
if (event.type == WebEventKeyDown) {
// FIXME: This is only for staging purposes.
@@ -2661,6 +2682,22 @@
else
[[UIKeyboardImpl sharedInstance] didHandleWebKeyEvent];
}
+
+ if (eventWasHandled)
+ return;
+
+ if (![event isKindOfClass:[WKWebEvent class]])
+ return;
+
+ // Resending the event may destroy this WKContentView.
+ RetainPtr<WKContentView> protector(self);
+
+ // 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.
+ ASSERT(!_uiEventBeingResent);
+ _uiEventBeingResent = [(WKWebEvent *)event uiEvent];
+ [super _handleKeyUIEvent:_uiEventBeingResent.get()];
+ _uiEventBeingResent = nil;
}
- (BOOL)_interpretKeyEvent:(WebIOSEvent *)event isCharEvent:(BOOL)isCharEvent
@@ -2673,6 +2710,7 @@
static const unsigned kWebSpaceKey = 0x20;
BOOL contentEditable = _page->editorState().isContentEditable;
+ WebCore::FloatRect unobscuredContentRect = _page->unobscuredContentRect();
if (!contentEditable && event.isTabKey)
return NO;
@@ -2688,29 +2726,29 @@
scrollOffset.setX(-Scrollbar::pixelsPerLineStep());
else if ([charactersIgnoringModifiers isEqualToString:UIKeyInputUpArrow]) {
if (option)
- scrollOffset.setY(-_page->unobscuredContentRect().height());
+ scrollOffset.setY(-unobscuredContentRect.height());
else if (command)
- scrollOffset.setY(-[self bounds].size.height);
+ scrollOffset.setY(-self.bounds.size.height);
else
scrollOffset.setY(-Scrollbar::pixelsPerLineStep());
} else if ([charactersIgnoringModifiers isEqualToString:UIKeyInputRightArrow])
scrollOffset.setX(Scrollbar::pixelsPerLineStep());
else if ([charactersIgnoringModifiers isEqualToString:UIKeyInputDownArrow]) {
if (option)
- scrollOffset.setY(_page->unobscuredContentRect().height());
+ scrollOffset.setY(unobscuredContentRect.height());
else if (command)
- scrollOffset.setY([self bounds].size.height);
+ scrollOffset.setY(self.bounds.size.height);
else
scrollOffset.setY(Scrollbar::pixelsPerLineStep());
} else if ([charactersIgnoringModifiers isEqualToString:UIKeyInputPageDown])
- scrollOffset.setY(_page->unobscuredContentRect().height());
+ scrollOffset.setY(unobscuredContentRect.height());
else if ([charactersIgnoringModifiers isEqualToString:UIKeyInputPageUp])
- scrollOffset.setY(-_page->unobscuredContentRect().height());
+ scrollOffset.setY(-unobscuredContentRect.height());
else
shouldScroll = NO;
if (shouldScroll) {
- [_webView _scrollByOffset:scrollOffset];
+ [_webView _scrollByContentOffset:scrollOffset];
return YES;
}
@@ -2735,7 +2773,8 @@
case kWebSpaceKey:
if (!contentEditable) {
- [_webView _scrollByOffset:FloatPoint(0, shift ? -_page->unobscuredContentRect().height() : _page->unobscuredContentRect().height())];
+ int pageStep = Scrollbar::pageStep(unobscuredContentRect.height(), self.bounds.size.height);
+ [_webView _scrollByContentOffset:FloatPoint(0, shift ? -pageStep : pageStep)];
return YES;
}
if (isCharEvent) {
Modified: branches/safari-601.1.46-branch/Tools/ChangeLog (192978 => 192979)
--- branches/safari-601.1.46-branch/Tools/ChangeLog 2015-12-02 23:04:28 UTC (rev 192978)
+++ branches/safari-601.1.46-branch/Tools/ChangeLog 2015-12-02 23:04:34 UTC (rev 192979)
@@ -1,3 +1,31 @@
+2015-12-01 Matthew Hanson <matthew_han...@apple.com>
+
+ Merge r192432. rdar://problem/23558672
+
+ 2015-11-13 Tim Horton <timothy_hor...@apple.com>
+
+ Hardware keyboard spacebar scrolls too far on iOS
+ https://bugs.webkit.org/show_bug.cgi?id=151227
+ <rdar://problem/23500681>
+
+ Reviewed by Simon Fraser.
+
+ * WebKitTestRunner/UIScriptContext/Bindings/UIScriptController.idl:
+ * WebKitTestRunner/UIScriptContext/UIScriptContext.h:
+ * WebKitTestRunner/UIScriptContext/UIScriptController.cpp:
+ (WTR::UIScriptController::setDidEndScrollingCallback):
+ (WTR::UIScriptController::didEndScrollingCallback):
+ (WTR::UIScriptController::platformSetDidEndScrollingCallback):
+ * WebKitTestRunner/UIScriptContext/UIScriptController.h:
+ * WebKitTestRunner/cocoa/TestRunnerWKWebView.h:
+ * WebKitTestRunner/cocoa/TestRunnerWKWebView.mm:
+ (-[TestRunnerWKWebView dealloc]):
+ (-[TestRunnerWKWebView _didFinishScrolling]):
+ * WebKitTestRunner/ios/UIScriptControllerIOS.mm:
+ (WTR::UIScriptController::platformSetDidEndScrollingCallback):
+ (WTR::UIScriptController::platformClearAllCallbacks):
+ Expose _didFinishScrolling on WKWebView to the UIScriptController.
+
2015-10-27 Matthew Hanson <matthew_han...@apple.com>
Merge r191395. rdar://problem/22846455
