- Revision
- 228253
- Author
- [email protected]
- Date
- 2018-02-07 17:27:04 -0800 (Wed, 07 Feb 2018)
Log Message
Evernote device management web view sometimes displays at the wrong scale
https://bugs.webkit.org/show_bug.cgi?id=182590
<rdar://problem/36633687>
Reviewed by Simon Fraser.
Evernote implements the WKWebView's scroll view's delegate method
viewForZoomingInScrollView: and returns nil. This results in
WKScrollView's zoomScale always returning 1, no matter what the
WKContentView's actual scale is. This will result in us never updating
the WKContentView's scale to 1. When loading a page that has a few
scale changes during load but ends up at scale 1, we get stuck at whatever
intermediate scale immediately preceded settling on 1.
Fix this by not forwarding viewForZoomingInScrollView: to the external
WKScrollView delegate; we are in charge of the contents of the scroll
view (including which view scrollView's zoomScale should track), and
overriding viewForZoomingInScrollView: is only ever going to lead to
a broken WebKit.
* UIProcess/ios/WKScrollView.mm:
(shouldForwardScrollViewDelegateMethodToExternalDelegate):
(-[WKScrollViewDelegateForwarder forwardInvocation:]):
(-[WKScrollViewDelegateForwarder forwardingTargetForSelector:]):
* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/ios/WKScrollViewDelegate.mm: Renamed from Tools/TestWebKitAPI/Tests/ios/WKScrollViewDelegateCrash.mm.
(-[TestDelegateForScrollView dealloc]):
(TestWebKitAPI::TEST):
(-[WKScrollViewDelegateWithViewForZoomingOverridden viewForZoomingInScrollView:]):
Add a test that failed before the change that ensures that we don't
consult the external delegate for viewForZoomingInScrollView:, and that
we succesfully update the scale even if it matches that of the view
the external delegate returns for viewForZoomingInScrollView:.
Modified Paths
Added Paths
Removed Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (228252 => 228253)
--- trunk/Source/WebKit/ChangeLog 2018-02-08 01:25:06 UTC (rev 228252)
+++ trunk/Source/WebKit/ChangeLog 2018-02-08 01:27:04 UTC (rev 228253)
@@ -1,3 +1,30 @@
+2018-02-07 Tim Horton <[email protected]>
+
+ Evernote device management web view sometimes displays at the wrong scale
+ https://bugs.webkit.org/show_bug.cgi?id=182590
+ <rdar://problem/36633687>
+
+ Reviewed by Simon Fraser.
+
+ Evernote implements the WKWebView's scroll view's delegate method
+ viewForZoomingInScrollView: and returns nil. This results in
+ WKScrollView's zoomScale always returning 1, no matter what the
+ WKContentView's actual scale is. This will result in us never updating
+ the WKContentView's scale to 1. When loading a page that has a few
+ scale changes during load but ends up at scale 1, we get stuck at whatever
+ intermediate scale immediately preceded settling on 1.
+
+ Fix this by not forwarding viewForZoomingInScrollView: to the external
+ WKScrollView delegate; we are in charge of the contents of the scroll
+ view (including which view scrollView's zoomScale should track), and
+ overriding viewForZoomingInScrollView: is only ever going to lead to
+ a broken WebKit.
+
+ * UIProcess/ios/WKScrollView.mm:
+ (shouldForwardScrollViewDelegateMethodToExternalDelegate):
+ (-[WKScrollViewDelegateForwarder forwardInvocation:]):
+ (-[WKScrollViewDelegateForwarder forwardingTargetForSelector:]):
+
2018-02-07 Wenson Hsieh <[email protected]>
[Extra zoom mode] Delegate scrolling from the content view to input view controllers
Modified: trunk/Source/WebKit/UIProcess/ios/WKScrollView.mm (228252 => 228253)
--- trunk/Source/WebKit/UIProcess/ios/WKScrollView.mm 2018-02-08 01:25:06 UTC (rev 228252)
+++ trunk/Source/WebKit/UIProcess/ios/WKScrollView.mm 2018-02-08 01:27:04 UTC (rev 228253)
@@ -79,12 +79,23 @@
return [super respondsToSelector:aSelector] || [_internalDelegate respondsToSelector:aSelector] || [_externalDelegate.get() respondsToSelector:aSelector];
}
+static BOOL shouldForwardScrollViewDelegateMethodToExternalDelegate(SEL selector)
+{
+ // We cannot forward viewForZoomingInScrollView: to the external delegate, because WebKit
+ // owns the content of the scroll view, and depends on viewForZoomingInScrollView being the
+ // content view. Any other view returned by the external delegate will break our behavior.
+ if (sel_isEqual(selector, @selector(viewForZoomingInScrollView:)))
+ return NO;
+
+ return YES;
+}
+
- (void)forwardInvocation:(NSInvocation *)anInvocation
{
auto externalDelegate = _externalDelegate.get();
SEL aSelector = [anInvocation selector];
BOOL internalDelegateWillRespond = [_internalDelegate respondsToSelector:aSelector];
- BOOL externalDelegateWillRespond = [externalDelegate respondsToSelector:aSelector];
+ BOOL externalDelegateWillRespond = shouldForwardScrollViewDelegateMethodToExternalDelegate(aSelector) && [externalDelegate respondsToSelector:aSelector];
if (internalDelegateWillRespond && externalDelegateWillRespond)
[_internalDelegate _willInvokeUIScrollViewDelegateCallback];
@@ -104,7 +115,7 @@
- (id)forwardingTargetForSelector:(SEL)aSelector
{
BOOL internalDelegateWillRespond = [_internalDelegate respondsToSelector:aSelector];
- BOOL externalDelegateWillRespond = [_externalDelegate.get() respondsToSelector:aSelector];
+ BOOL externalDelegateWillRespond = shouldForwardScrollViewDelegateMethodToExternalDelegate(aSelector) && [_externalDelegate.get() respondsToSelector:aSelector];
if (internalDelegateWillRespond && !externalDelegateWillRespond)
return _internalDelegate;
Modified: trunk/Tools/ChangeLog (228252 => 228253)
--- trunk/Tools/ChangeLog 2018-02-08 01:25:06 UTC (rev 228252)
+++ trunk/Tools/ChangeLog 2018-02-08 01:27:04 UTC (rev 228253)
@@ -1,3 +1,21 @@
+2018-02-07 Tim Horton <[email protected]>
+
+ Evernote device management web view sometimes displays at the wrong scale
+ https://bugs.webkit.org/show_bug.cgi?id=182590
+ <rdar://problem/36633687>
+
+ Reviewed by Simon Fraser.
+
+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+ * TestWebKitAPI/Tests/ios/WKScrollViewDelegate.mm: Renamed from Tools/TestWebKitAPI/Tests/ios/WKScrollViewDelegateCrash.mm.
+ (-[TestDelegateForScrollView dealloc]):
+ (TestWebKitAPI::TEST):
+ (-[WKScrollViewDelegateWithViewForZoomingOverridden viewForZoomingInScrollView:]):
+ Add a test that failed before the change that ensures that we don't
+ consult the external delegate for viewForZoomingInScrollView:, and that
+ we succesfully update the scale even if it matches that of the view
+ the external delegate returns for viewForZoomingInScrollView:.
+
2018-02-07 Wenson Hsieh <[email protected]>
REGRESSION(r226396): File paths are inserted when dropping image files
Modified: trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (228252 => 228253)
--- trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2018-02-08 01:25:06 UTC (rev 228252)
+++ trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2018-02-08 01:27:04 UTC (rev 228253)
@@ -267,7 +267,7 @@
5CB5B3C21FFC55CF00C27BB0 /* FrameHandleSerialization.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5CB5B3BD1FFC517E00C27BB0 /* FrameHandleSerialization.mm */; };
5CE354D91E70DA5C00BEFE3B /* WKContentExtensionStore.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5CE354D81E70D9C300BEFE3B /* WKContentExtensionStore.mm */; };
5CEAB5E11FA939F400A77FAA /* _WKInputDelegate.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5CEAB5DF1FA937CB00A77FAA /* _WKInputDelegate.mm */; };
- 5E4B1D2E1D404C6100053621 /* WKScrollViewDelegateCrash.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5E4B1D2C1D404C6100053621 /* WKScrollViewDelegateCrash.mm */; };
+ 5E4B1D2E1D404C6100053621 /* WKScrollViewDelegate.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5E4B1D2C1D404C6100053621 /* WKScrollViewDelegate.mm */; };
631EFFF61E7B5E8D00D2EBB8 /* Geolocation.mm in Sources */ = {isa = PBXBuildFile; fileRef = 631EFFF51E7B5E8D00D2EBB8 /* Geolocation.mm */; };
634910E01E9D3FF300880309 /* CoreLocation.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 634910DF1E9D3FF300880309 /* CoreLocation.framework */; };
6354F4D11F7C3AB500D89DF3 /* ApplicationManifestParser.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 6354F4D01F7C3AB500D89DF3 /* ApplicationManifestParser.cpp */; };
@@ -1438,7 +1438,7 @@
5CB5B3BD1FFC517E00C27BB0 /* FrameHandleSerialization.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = FrameHandleSerialization.mm; sourceTree = "<group>"; };
5CE354D81E70D9C300BEFE3B /* WKContentExtensionStore.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WKContentExtensionStore.mm; sourceTree = "<group>"; };
5CEAB5DF1FA937CB00A77FAA /* _WKInputDelegate.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = _WKInputDelegate.mm; sourceTree = "<group>"; };
- 5E4B1D2C1D404C6100053621 /* WKScrollViewDelegateCrash.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = WKScrollViewDelegateCrash.mm; path = ../ios/WKScrollViewDelegateCrash.mm; sourceTree = "<group>"; };
+ 5E4B1D2C1D404C6100053621 /* WKScrollViewDelegate.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = WKScrollViewDelegate.mm; path = ../ios/WKScrollViewDelegate.mm; sourceTree = "<group>"; };
631EFFF51E7B5E8D00D2EBB8 /* Geolocation.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = Geolocation.mm; sourceTree = "<group>"; };
634910DF1E9D3FF300880309 /* CoreLocation.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = CoreLocation.framework; path = System/Library/Frameworks/CoreLocation.framework; sourceTree = SDKROOT; };
6354F4D01F7C3AB500D89DF3 /* ApplicationManifestParser.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = ApplicationManifestParser.cpp; sourceTree = "<group>"; };
@@ -2172,7 +2172,7 @@
2D00065D1C1F58940088E6A7 /* WKPDFViewResizeCrash.mm */,
2D21FE581F04642800B58E7D /* WKPDFViewStablePresentationUpdateCallback.mm */,
44817A2E1F0486BF00003810 /* WKRequestActivatedElementInfo.mm */,
- 5E4B1D2C1D404C6100053621 /* WKScrollViewDelegateCrash.mm */,
+ 5E4B1D2C1D404C6100053621 /* WKScrollViewDelegate.mm */,
51C683DD1EA134DB00650183 /* WKURLSchemeHandler-1.mm */,
5182C22D1F2BCB410059BA7C /* WKURLSchemeHandler-leaks.mm */,
371195AA1FE5797700A1FB92 /* WKWebViewAlwaysShowsScroller.mm */,
@@ -3516,6 +3516,7 @@
5C0BF88D1DD5964D00B00328 /* MemoryPressureHandler.mm in Sources */,
7C83E0B71D0A64B800FEBCF3 /* MenuTypesForMouseEvents.cpp in Sources */,
5C0BF8941DD599C900B00328 /* MenuTypesForMouseEvents.mm in Sources */,
+ 5165FE04201EE620009F7EC3 /* MessagePortProviders.mm in Sources */,
A5B149DE1F5A19EA00C6DAFF /* MIMETypeRegistry.cpp in Sources */,
51CD1C6C1B38CE4300142CA5 /* ModalAlerts.mm in Sources */,
7C83E0B61D0A64B300FEBCF3 /* ModalAlertsSPI.cpp in Sources */,
@@ -3560,7 +3561,6 @@
7CCE7F0C1A411AE600447C4C /* PrivateBrowsingPushStateNoHistoryCallback.cpp in Sources */,
4647B1261EBA3B850041D7EF /* ProcessDidTerminate.cpp in Sources */,
7C83E0C11D0A652F00FEBCF3 /* ProvisionalURLNotChange.mm in Sources */,
- 5165FE04201EE620009F7EC3 /* MessagePortProviders.mm in Sources */,
7CCE7EC81A411A7E00447C4C /* PublicSuffix.mm in Sources */,
7C83E0C21D0A653500FEBCF3 /* QuickLook.mm in Sources */,
7CCE7F0D1A411AE600447C4C /* ReloadPageAfterCrash.cpp in Sources */,
@@ -3680,7 +3680,7 @@
7CCE7F211A411AE600447C4C /* WKPreferences.cpp in Sources */,
44817A2F1F0486BF00003810 /* WKRequestActivatedElementInfo.mm in Sources */,
7C83E0B51D0A649300FEBCF3 /* WKRetainPtr.cpp in Sources */,
- 5E4B1D2E1D404C6100053621 /* WKScrollViewDelegateCrash.mm in Sources */,
+ 5E4B1D2E1D404C6100053621 /* WKScrollViewDelegate.mm in Sources */,
7CCE7F221A411AE600447C4C /* WKString.cpp in Sources */,
7CCE7F1E1A411AE600447C4C /* WKStringJSString.cpp in Sources */,
2D4CF8BD1D8360CC0001CE8D /* WKThumbnailView.mm in Sources */,
Copied: trunk/Tools/TestWebKitAPI/Tests/ios/WKScrollViewDelegate.mm (from rev 228251, trunk/Tools/TestWebKitAPI/Tests/ios/WKScrollViewDelegateCrash.mm) (0 => 228253)
--- trunk/Tools/TestWebKitAPI/Tests/ios/WKScrollViewDelegate.mm (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/ios/WKScrollViewDelegate.mm 2018-02-08 01:27:04 UTC (rev 228253)
@@ -0,0 +1,121 @@
+/*
+ * Copyright (C) 2016-2018 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#import "config.h"
+
+#if PLATFORM(IOS)
+
+#import "PlatformUtilities.h"
+#import "Test.h"
+#import "TestWKWebView.h"
+#import <WebKit/WKWebView.h>
+
+// WKScrollViewDelegateCrash
+
+static bool delegateIsDeallocated;
+
+@interface TestDelegateForScrollView : NSObject <UIScrollViewDelegate>
+
+@end
+
+@implementation TestDelegateForScrollView
+
+- (void)dealloc
+{
+ delegateIsDeallocated = true;
+ [super dealloc];
+}
+
+@end
+
+namespace TestWebKitAPI {
+
+TEST(WKWebView, WKScrollViewDelegateCrash)
+{
+ WKWebView *webView = [[WKWebView alloc] init];
+ TestDelegateForScrollView *delegateForScrollView = [[TestDelegateForScrollView alloc] init];
+ @autoreleasepool {
+ webView.scrollView.delegate = delegateForScrollView;
+ }
+ delegateIsDeallocated = false;
+ [delegateForScrollView release];
+ TestWebKitAPI::Util::run(&delegateIsDeallocated);
+
+ EXPECT_NULL(webView.scrollView.delegate);
+}
+
+}
+
+// WKScrollViewDelegateCannotOverrideViewForZooming
+
+static BOOL didCallViewForZoomingInScrollView;
+
+@interface WKScrollViewDelegateWithViewForZoomingOverridden : NSObject <UIScrollViewDelegate>
+@property (nonatomic, assign) CGFloat overrideScale;
+@end
+
+@implementation WKScrollViewDelegateWithViewForZoomingOverridden
+
+- (UIView *)viewForZoomingInScrollView:(UIScrollView *)scrollView
+{
+ didCallViewForZoomingInScrollView = true;
+
+ UIView *fakeZoomingView = [[UIView alloc] init];
+ fakeZoomingView.layer.affineTransform = CGAffineTransformMakeScale(_overrideScale, _overrideScale);
+ return fakeZoomingView;
+}
+
+@end
+
+namespace TestWebKitAPI {
+
+TEST(WKWebView, WKScrollViewDelegateCannotOverrideViewForZooming)
+{
+ TestWKWebView *webView = [[TestWKWebView alloc] init];
+ WKScrollViewDelegateWithViewForZoomingOverridden *delegateForScrollView = [[WKScrollViewDelegateWithViewForZoomingOverridden alloc] init];
+ @autoreleasepool {
+ webView.scrollView.delegate = delegateForScrollView;
+ }
+
+ [webView synchronouslyLoadHTMLString:@"<meta name='viewport' content='initial-scale=1'>"];
+ [webView waitForNextPresentationUpdate];
+
+ // Have WKScrollView's external delegate return a view with scale=2 from viewForZoomingInScrollView.
+ delegateForScrollView.overrideScale = 2;
+
+ [webView synchronouslyLoadHTMLString:@"<meta name='viewport' content='initial-scale=2'>"];
+ [webView waitForNextPresentationUpdate];
+
+ @autoreleasepool {
+ webView.scrollView.delegate = nil;
+ }
+
+ EXPECT_FALSE(didCallViewForZoomingInScrollView);
+ EXPECT_EQ(2, [[webView scrollView] zoomScale]);
+}
+
+}
+
+#endif
Deleted: trunk/Tools/TestWebKitAPI/Tests/ios/WKScrollViewDelegateCrash.mm (228252 => 228253)
--- trunk/Tools/TestWebKitAPI/Tests/ios/WKScrollViewDelegateCrash.mm 2018-02-08 01:25:06 UTC (rev 228252)
+++ trunk/Tools/TestWebKitAPI/Tests/ios/WKScrollViewDelegateCrash.mm 2018-02-08 01:27:04 UTC (rev 228253)
@@ -1,68 +0,0 @@
-/*
- * Copyright (C) 2016 Apple Inc. All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
- * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
- * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
- * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
- * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
- * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
- * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
- * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
- * THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#import "config.h"
-
-#if PLATFORM(IOS)
-
-#import "PlatformUtilities.h"
-#import "Test.h"
-#import <WebKit/WKWebView.h>
-
-static bool delegateIsDeallocated;
-
-@interface TestDelegateForScrollView : NSObject <UIScrollViewDelegate>
-
-@end
-
-@implementation TestDelegateForScrollView
-
-- (void)dealloc
-{
- delegateIsDeallocated = true;
- [super dealloc];
-}
-
-@end
-
-namespace TestWebKitAPI {
-
-TEST(WKWebView, WKScrollViewDelegateCrash)
-{
- WKWebView *webView = [[WKWebView alloc] init];
- TestDelegateForScrollView *delegateForScrollView = [[TestDelegateForScrollView alloc] init];
- @autoreleasepool {
- webView.scrollView.delegate = delegateForScrollView;
- }
- delegateIsDeallocated = false;
- [delegateForScrollView release];
- TestWebKitAPI::Util::run(&delegateIsDeallocated);
-
- EXPECT_NULL(webView.scrollView.delegate);
-}
-
-}
-
-#endif
