- Revision
- 266021
- Author
- wenson_hs...@apple.com
- Date
- 2020-08-21 17:52:47 -0700 (Fri, 21 Aug 2020)
Log Message
[macOS] Unable to copy text from the function browser panel in Numbers
https://bugs.webkit.org/show_bug.cgi?id=215740
<rdar://problem/65189303>
Reviewed by Darin Adler.
Source/WebKit:
On some versions of Xcode, using Interface Builder to set up WKWebViews causes the WKWebView to contain two
WKFlippedViews as subviews; one of these is created and added in the normal initialization path of the web view
underneath WebViewImpl's constructor, and the other is created and added underneath the superclass (NSView's)
`-initWithCoder:` implementation.
This causes issues when hit-testing WKWebView using the `-hitTest:` method, which is expected to return the
WKWebView itself instead of the inner WKFlippedView. The logic that tries to implement this behavior is in
`WebViewImpl::hitTest`, which calls into WKWebView's `-hitTest:` method and returns the WKWebView if the hit-
tested view ended up being equal to the `m_layerHostingView`. Since we end up with two layer hosting views (only
one of which is the real `m_layerHostingView`), we fail the check and end up returning the other WKFlippedView.
In the context of this bug, this erroneous hit-test causes AppKit to ask if the WKFlippedView can become the
first responder upon mousedown (which it cannot), and so the window's first responder remains the same (instead
of changing to WKWebView).
To fix this, adjust the constructor of WebViewImpl to avoid making an extra layer hosting view in the case where
decoding the WKWebView already initialized and placed the layer hosting view, and just use the existing one
instead.
Test: WKWebView.HitTestAfterInitializingFromCoder
* UIProcess/Cocoa/WebViewImpl.mm:
(-[WKFlippedView initWithFrame:]):
(-[WKFlippedView initWithCoder:]):
(-[WKFlippedView _commonInitialize]):
Override both designated initialization codepaths so that we call the `-_commonInitialize` helper, which sets
the autoresizing mask. This was previously called immediately after creating the WKFlippedView in the
constructor of WebViewImpl.
(WebKit::WebViewImpl::WebViewImpl):
Tools:
Add a new API test that serializes a WKWebView, then deserializes it, and then verifies that calling `-hitTest:`
on the deserialized `WKWebView` grabs the `WKWebView` itself rather than the `WKFlippedView` underneath it.
* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/mac/WKWebViewCoders.mm: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (266020 => 266021)
--- trunk/Source/WebKit/ChangeLog 2020-08-22 00:20:51 UTC (rev 266020)
+++ trunk/Source/WebKit/ChangeLog 2020-08-22 00:52:47 UTC (rev 266021)
@@ -1,3 +1,42 @@
+2020-08-21 Wenson Hsieh <wenson_hs...@apple.com>
+
+ [macOS] Unable to copy text from the function browser panel in Numbers
+ https://bugs.webkit.org/show_bug.cgi?id=215740
+ <rdar://problem/65189303>
+
+ Reviewed by Darin Adler.
+
+ On some versions of Xcode, using Interface Builder to set up WKWebViews causes the WKWebView to contain two
+ WKFlippedViews as subviews; one of these is created and added in the normal initialization path of the web view
+ underneath WebViewImpl's constructor, and the other is created and added underneath the superclass (NSView's)
+ `-initWithCoder:` implementation.
+
+ This causes issues when hit-testing WKWebView using the `-hitTest:` method, which is expected to return the
+ WKWebView itself instead of the inner WKFlippedView. The logic that tries to implement this behavior is in
+ `WebViewImpl::hitTest`, which calls into WKWebView's `-hitTest:` method and returns the WKWebView if the hit-
+ tested view ended up being equal to the `m_layerHostingView`. Since we end up with two layer hosting views (only
+ one of which is the real `m_layerHostingView`), we fail the check and end up returning the other WKFlippedView.
+ In the context of this bug, this erroneous hit-test causes AppKit to ask if the WKFlippedView can become the
+ first responder upon mousedown (which it cannot), and so the window's first responder remains the same (instead
+ of changing to WKWebView).
+
+ To fix this, adjust the constructor of WebViewImpl to avoid making an extra layer hosting view in the case where
+ decoding the WKWebView already initialized and placed the layer hosting view, and just use the existing one
+ instead.
+
+ Test: WKWebView.HitTestAfterInitializingFromCoder
+
+ * UIProcess/Cocoa/WebViewImpl.mm:
+ (-[WKFlippedView initWithFrame:]):
+ (-[WKFlippedView initWithCoder:]):
+ (-[WKFlippedView _commonInitialize]):
+
+ Override both designated initialization codepaths so that we call the `-_commonInitialize` helper, which sets
+ the autoresizing mask. This was previously called immediately after creating the WKFlippedView in the
+ constructor of WebViewImpl.
+
+ (WebKit::WebViewImpl::WebViewImpl):
+
2020-08-21 Andy Estes <aes...@apple.com>
IPC::encodeSharedBuffer combines SharedBuffer data segments when copying to SharedMemory
Modified: trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm (266020 => 266021)
--- trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm 2020-08-22 00:20:51 UTC (rev 266020)
+++ trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm 2020-08-22 00:52:47 UTC (rev 266021)
@@ -414,6 +414,25 @@
@implementation WKFlippedView
+- (instancetype)initWithFrame:(NSRect)frame
+{
+ if (self = [super initWithFrame:frame])
+ [self _commonInitialize];
+ return self;
+}
+
+- (instancetype)initWithCoder:(NSCoder *)coder
+{
+ if (self = [super initWithCoder:coder])
+ [self _commonInitialize];
+ return self;
+}
+
+- (void)_commonInitialize
+{
+ [self setAutoresizingMask:NSViewWidthSizable | NSViewHeightSizable];
+}
+
- (BOOL)isFlipped
{
return YES;
@@ -1377,11 +1396,21 @@
[view addTrackingArea:m_primaryTrackingArea.get()];
- // Create an NSView that will host our layer tree.
- m_layerHostingView = adoptNS([[WKFlippedView alloc] initWithFrame:[m_view bounds]]);
- [m_layerHostingView setAutoresizingMask:NSViewWidthSizable | NSViewHeightSizable];
+ for (NSView *subview in view.subviews) {
+ if ([subview isKindOfClass:WKFlippedView.class]) {
+ // A layer hosting view may have already been created and added to the view hierarchy
+ // in the process of initializing the WKWebView from an NSCoder.
+ m_layerHostingView = (WKFlippedView *)subview;
+ [m_layerHostingView setFrame:[m_view bounds]];
+ break;
+ }
+ }
- [view addSubview:m_layerHostingView.get() positioned:NSWindowBelow relativeTo:nil];
+ if (!m_layerHostingView) {
+ // Create an NSView that will host our layer tree.
+ m_layerHostingView = adoptNS([[WKFlippedView alloc] initWithFrame:[m_view bounds]]);
+ [view addSubview:m_layerHostingView.get() positioned:NSWindowBelow relativeTo:nil];
+ }
// Create a root layer that will back the NSView.
RetainPtr<CALayer> layer = adoptNS([[CALayer alloc] init]);
Modified: trunk/Tools/ChangeLog (266020 => 266021)
--- trunk/Tools/ChangeLog 2020-08-22 00:20:51 UTC (rev 266020)
+++ trunk/Tools/ChangeLog 2020-08-22 00:52:47 UTC (rev 266021)
@@ -1,3 +1,17 @@
+2020-08-21 Wenson Hsieh <wenson_hs...@apple.com>
+
+ [macOS] Unable to copy text from the function browser panel in Numbers
+ https://bugs.webkit.org/show_bug.cgi?id=215740
+ <rdar://problem/65189303>
+
+ Reviewed by Darin Adler.
+
+ Add a new API test that serializes a WKWebView, then deserializes it, and then verifies that calling `-hitTest:`
+ on the deserialized `WKWebView` grabs the `WKWebView` itself rather than the `WKFlippedView` underneath it.
+
+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+ * TestWebKitAPI/Tests/mac/WKWebViewCoders.mm: Added.
+
2020-08-21 Jonathan Bedard <jbed...@apple.com>
Regression (r265942): Layout-tests are hanging on iOS EWS
Modified: trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (266020 => 266021)
--- trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2020-08-22 00:20:51 UTC (rev 266020)
+++ trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2020-08-22 00:52:47 UTC (rev 266021)
@@ -1100,6 +1100,7 @@
F41AB9AA1EF4696B0083FA08 /* textarea-to-input.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F41AB9951EF4692C0083FA08 /* textarea-to-input.html */; };
F422A3E9235ACA9B00CF00CA /* ClipboardTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = F422A3E8235ACA9B00CF00CA /* ClipboardTests.mm */; };
F422A3EB235BB16200CF00CA /* clipboard.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F422A3EA235BB14500CF00CA /* clipboard.html */; };
+ F425831624F07CA5006B985D /* WKWebViewCoders.mm in Sources */ = {isa = PBXBuildFile; fileRef = F425831524F07CA5006B985D /* WKWebViewCoders.mm */; };
F42BD7D9245CC508001E207A /* attributedStringNewlineAtEndOfDocument.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F42BD7D8245CC4E6001E207A /* attributedStringNewlineAtEndOfDocument.html */; };
F42D634422A1729F00D2FB3A /* AutocorrectionTestsIOS.mm in Sources */ = {isa = PBXBuildFile; fileRef = F42D634322A1729F00D2FB3A /* AutocorrectionTestsIOS.mm */; };
F42DA5161D8CEFE400336F40 /* large-input-field-focus-onload.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F42DA5151D8CEFDB00336F40 /* large-input-field-focus-onload.html */; };
@@ -2806,6 +2807,7 @@
F41AB99E1EF4692C0083FA08 /* div-and-large-image.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "div-and-large-image.html"; sourceTree = "<group>"; };
F422A3E8235ACA9B00CF00CA /* ClipboardTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = ClipboardTests.mm; sourceTree = "<group>"; };
F422A3EA235BB14500CF00CA /* clipboard.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = clipboard.html; sourceTree = "<group>"; };
+ F425831524F07CA5006B985D /* WKWebViewCoders.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = WKWebViewCoders.mm; sourceTree = "<group>"; };
F42BD7D8245CC4E6001E207A /* attributedStringNewlineAtEndOfDocument.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = attributedStringNewlineAtEndOfDocument.html; sourceTree = "<group>"; };
F42D634322A1729F00D2FB3A /* AutocorrectionTestsIOS.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = AutocorrectionTestsIOS.mm; sourceTree = "<group>"; };
F42DA5151D8CEFDB00336F40 /* large-input-field-focus-onload.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; name = "large-input-field-focus-onload.html"; path = "Tests/WebKitCocoa/large-input-field-focus-onload.html"; sourceTree = SOURCE_ROOT; };
@@ -4440,6 +4442,7 @@
CE32C7C718184C4900CD8C28 /* WillPerformClientRedirectToURLCrash.mm */,
1A7BFC0A171A0BDB00BC5F64 /* WillSendSubmitEvent.mm */,
A5E2027215B2181900C13E14 /* WindowlessWebViewWithMedia.mm */,
+ F425831524F07CA5006B985D /* WKWebViewCoders.mm */,
F4FA917F1E61849B007B8C1D /* WKWebViewMacEditingTests.mm */,
);
path = mac;
@@ -5497,6 +5500,7 @@
514958BE1F7427AC00E87BAD /* WKWebViewAutofillTests.mm in Sources */,
2EFF06D71D8AF34A0004BB30 /* WKWebViewCandidateTests.mm in Sources */,
CDD68F0D22C18317000CF0AE /* WKWebViewCloseAllMediaPresentations.mm in Sources */,
+ F425831624F07CA5006B985D /* WKWebViewCoders.mm in Sources */,
5CB3CE391FA1697F00C3A2D6 /* WKWebViewConfiguration.mm in Sources */,
7C417F331D19E14800B8EF53 /* WKWebViewDefaultNavigationDelegate.mm in Sources */,
46E66A901F0D75590026D83C /* WKWebViewDiagnosticLogging.mm in Sources */,
Added: trunk/Tools/TestWebKitAPI/Tests/mac/WKWebViewCoders.mm (0 => 266021)
--- trunk/Tools/TestWebKitAPI/Tests/mac/WKWebViewCoders.mm (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/mac/WKWebViewCoders.mm 2020-08-22 00:52:47 UTC (rev 266021)
@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2020 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"
+
+#import "TestWKWebView.h"
+#import <wtf/RetainPtr.h>
+
+TEST(WKWebView, HitTestAfterInitializingFromCoder)
+{
+ auto webViewToEncode = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
+ [webViewToEncode synchronouslyLoadTestPageNamed:@"simple"];
+
+ auto archiver = adoptNS([[NSKeyedArchiver alloc] init]);
+ [archiver encodeObject:webViewToEncode.get() forKey:@"WebView"];
+
+ auto unarchiver = adoptNS([[NSKeyedUnarchiver alloc] initForReadingFromData:[archiver encodedData] error:nil]);
+ [unarchiver setRequiresSecureCoding:NO];
+
+ WKWebView *decodedWebView = [unarchiver decodeObjectForKey:@"WebView"];
+ EXPECT_EQ(decodedWebView, [decodedWebView hitTest:NSMakePoint(400, 300)]);
+}
