- Revision
- 237131
- Author
- commit-qu...@webkit.org
- Date
- 2018-10-15 12:28:09 -0700 (Mon, 15 Oct 2018)
Log Message
Web Inspector: RDM: Toolbar hidden in when Inspector is docked to side.
https://bugs.webkit.org/show_bug.cgi?id=190545
rdar://problem/44674500
Patch by Remy Demarest <rdemar...@apple.com> on 2018-10-15
Reviewed by Brian Burg.
When the inspector is placed next to the web view it uses its _topContentInset and _totalHeightOfBanners
to lay out the inspector so it does not underlap the window toolbar, but this technique does not work when
in responsive design mode because of the different attachment view. This patch fixes the issue by using
-[NSWindow contentLayoutRect] to figure out the height of the inspector instead of relying on the content
insets of the web view. This requires observing -contentLayoutRect and ensure we only observe its changes
when the view is actually on the screen.
* UIProcess/WebInspectorProxy.h:
Declare helpers to add/remove observer on the attached inspector window.
* UIProcess/mac/WKInspectorViewController.h:
* UIProcess/mac/WKInspectorViewController.mm:
(-[WKInspectorViewController inspectorWKWebView:willMoveToWindow:]):
(-[WKInspectorViewController inspectorWKWebViewDidMoveToWindow:]):
* UIProcess/mac/WKInspectorWKWebView.h:
* UIProcess/mac/WKInspectorWKWebView.mm:
(-[WKInspectorWKWebView viewWillMoveToWindow:]):
(-[WKInspectorWKWebView viewDidMoveToWindow]):
* UIProcess/mac/WebInspectorProxyMac.mm:
(-[WKWebInspectorProxyObjCAdapter observeValueForKeyPath:ofObject:change:context:]): Update inspector layout
whenever the contentLayoutRect changes. Except when live resizing since the attachment view also sends
notifications at the same time.
(-[WKWebInspectorProxyObjCAdapter inspectorViewController:willMoveToWindow:]):
(-[WKWebInspectorProxyObjCAdapter inspectorViewControllerDidMoveToWindow:]):
(WebKit::WebInspectorProxy::attachmentWillMoveFromWindow): Remove the observer only if we set it up before.
(WebKit::WebInspectorProxy::attachmentDidMoveToWindow): Set up the observer and immediately update the frame
of the inspector since it just moved to its final destination.
(WebKit::WebInspectorProxy::inspectedViewFrameDidChange): Adjust the frame of the attached inspector based
on the contentLayoutRect of the window rather than the topContentInset of the web view.
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (237130 => 237131)
--- trunk/Source/WebKit/ChangeLog 2018-10-15 19:14:50 UTC (rev 237130)
+++ trunk/Source/WebKit/ChangeLog 2018-10-15 19:28:09 UTC (rev 237131)
@@ -1,3 +1,43 @@
+2018-10-15 Remy Demarest <rdemar...@apple.com>
+
+ Web Inspector: RDM: Toolbar hidden in when Inspector is docked to side.
+ https://bugs.webkit.org/show_bug.cgi?id=190545
+ rdar://problem/44674500
+
+ Reviewed by Brian Burg.
+
+ When the inspector is placed next to the web view it uses its _topContentInset and _totalHeightOfBanners
+ to lay out the inspector so it does not underlap the window toolbar, but this technique does not work when
+ in responsive design mode because of the different attachment view. This patch fixes the issue by using
+ -[NSWindow contentLayoutRect] to figure out the height of the inspector instead of relying on the content
+ insets of the web view. This requires observing -contentLayoutRect and ensure we only observe its changes
+ when the view is actually on the screen.
+
+ * UIProcess/WebInspectorProxy.h:
+ Declare helpers to add/remove observer on the attached inspector window.
+
+ * UIProcess/mac/WKInspectorViewController.h:
+ * UIProcess/mac/WKInspectorViewController.mm:
+ (-[WKInspectorViewController inspectorWKWebView:willMoveToWindow:]):
+ (-[WKInspectorViewController inspectorWKWebViewDidMoveToWindow:]):
+
+ * UIProcess/mac/WKInspectorWKWebView.h:
+ * UIProcess/mac/WKInspectorWKWebView.mm:
+ (-[WKInspectorWKWebView viewWillMoveToWindow:]):
+ (-[WKInspectorWKWebView viewDidMoveToWindow]):
+
+ * UIProcess/mac/WebInspectorProxyMac.mm:
+ (-[WKWebInspectorProxyObjCAdapter observeValueForKeyPath:ofObject:change:context:]): Update inspector layout
+ whenever the contentLayoutRect changes. Except when live resizing since the attachment view also sends
+ notifications at the same time.
+ (-[WKWebInspectorProxyObjCAdapter inspectorViewController:willMoveToWindow:]):
+ (-[WKWebInspectorProxyObjCAdapter inspectorViewControllerDidMoveToWindow:]):
+ (WebKit::WebInspectorProxy::attachmentWillMoveFromWindow): Remove the observer only if we set it up before.
+ (WebKit::WebInspectorProxy::attachmentDidMoveToWindow): Set up the observer and immediately update the frame
+ of the inspector since it just moved to its final destination.
+ (WebKit::WebInspectorProxy::inspectedViewFrameDidChange): Adjust the frame of the attached inspector based
+ on the contentLayoutRect of the window rather than the topContentInset of the web view.
+
2018-10-15 Wenson Hsieh <wenson_hs...@apple.com>
Changing view scale should zoom to initial scale if the page is already at initial scale
Modified: trunk/Source/WebKit/UIProcess/WebInspectorProxy.h (237130 => 237131)
--- trunk/Source/WebKit/UIProcess/WebInspectorProxy.h 2018-10-15 19:14:50 UTC (rev 237130)
+++ trunk/Source/WebKit/UIProcess/WebInspectorProxy.h 2018-10-15 19:28:09 UTC (rev 237131)
@@ -114,6 +114,8 @@
void closeFrontendAfterInactivityTimerFired();
void attachmentViewDidChange(NSView *oldView, NSView *newView);
+ void attachmentWillMoveFromWindow(NSWindow *oldWindow);
+ void attachmentDidMoveToWindow(NSWindow *newWindow);
#endif
#if PLATFORM(GTK)
@@ -252,6 +254,7 @@
HashMap<String, RetainPtr<NSURL>> m_suggestedToActualURLMap;
RunLoop::Timer<WebInspectorProxy> m_closeFrontendAfterInactivityTimer;
String m_urlString;
+ bool m_isObservingContentLayoutRect { false };
#elif PLATFORM(GTK)
std::unique_ptr<WebInspectorProxyClient> m_client;
GtkWidget* m_inspectorView { nullptr };
Modified: trunk/Source/WebKit/UIProcess/mac/WKInspectorViewController.h (237130 => 237131)
--- trunk/Source/WebKit/UIProcess/mac/WKInspectorViewController.h 2018-10-15 19:14:50 UTC (rev 237130)
+++ trunk/Source/WebKit/UIProcess/mac/WKInspectorViewController.h 2018-10-15 19:28:09 UTC (rev 237131)
@@ -54,6 +54,8 @@
@optional
- (void)inspectorViewControllerInspectorDidCrash:(WKInspectorViewController *)inspectorViewController;
- (BOOL)inspectorViewControllerInspectorIsUnderTest:(WKInspectorViewController *)inspectorViewController;
+- (void)inspectorViewController:(WKInspectorViewController *)inspectorViewController willMoveToWindow:(NSWindow *)newWindow;
+- (void)inspectorViewControllerDidMoveToWindow:(WKInspectorViewController *)inspectorViewController;
@end
NS_ASSUME_NONNULL_END
Modified: trunk/Source/WebKit/UIProcess/mac/WKInspectorViewController.mm (237130 => 237131)
--- trunk/Source/WebKit/UIProcess/mac/WKInspectorViewController.mm 2018-10-15 19:14:50 UTC (rev 237130)
+++ trunk/Source/WebKit/UIProcess/mac/WKInspectorViewController.mm 2018-10-15 19:28:09 UTC (rev 237131)
@@ -251,6 +251,18 @@
_inspectedPage->reload(WebCore::ReloadOption::FromOrigin);
}
+- (void)inspectorWKWebView:(WKInspectorWKWebView *)webView willMoveToWindow:(NSWindow *)newWindow
+{
+ if (!!_delegate && [_delegate respondsToSelector:@selector(inspectorViewController:willMoveToWindow:)])
+ [_delegate inspectorViewController:self willMoveToWindow:newWindow];
+}
+
+- (void)inspectorWKWebViewDidMoveToWindow:(WKInspectorWKWebView *)webView
+{
+ if (!!_delegate && [_delegate respondsToSelector:@selector(inspectorViewControllerDidMoveToWindow:)])
+ [_delegate inspectorViewControllerDidMoveToWindow:self];
+}
+
@end
#endif // PLATFORM(MAC) && WK_API_ENABLED
Modified: trunk/Source/WebKit/UIProcess/mac/WKInspectorWKWebView.h (237130 => 237131)
--- trunk/Source/WebKit/UIProcess/mac/WKInspectorWKWebView.h 2018-10-15 19:14:50 UTC (rev 237130)
+++ trunk/Source/WebKit/UIProcess/mac/WKInspectorWKWebView.h 2018-10-15 19:28:09 UTC (rev 237131)
@@ -39,6 +39,8 @@
@required
- (void)inspectorWKWebViewReload:(WKInspectorWKWebView *)webView;
- (void)inspectorWKWebViewReloadFromOrigin:(WKInspectorWKWebView *)webView;
+- (void)inspectorWKWebView:(WKInspectorWKWebView *)webView willMoveToWindow:(NSWindow *)newWindow;
+- (void)inspectorWKWebViewDidMoveToWindow:(WKInspectorWKWebView *)webView;
@end
#endif // PLATFORM(MAC) && WK_API_ENABLED
Modified: trunk/Source/WebKit/UIProcess/mac/WKInspectorWKWebView.mm (237130 => 237131)
--- trunk/Source/WebKit/UIProcess/mac/WKInspectorWKWebView.mm 2018-10-15 19:14:50 UTC (rev 237130)
+++ trunk/Source/WebKit/UIProcess/mac/WKInspectorWKWebView.mm 2018-10-15 19:28:09 UTC (rev 237131)
@@ -60,6 +60,18 @@
[self.inspectorWKWebViewDelegate inspectorWKWebViewReloadFromOrigin:self];
}
+- (void)viewWillMoveToWindow:(NSWindow *)newWindow
+{
+ [super viewWillMoveToWindow:newWindow];
+ [self.inspectorWKWebViewDelegate inspectorWKWebView:self willMoveToWindow:newWindow];
+}
+
+- (void)viewDidMoveToWindow
+{
+ [super viewDidMoveToWindow];
+ [self.inspectorWKWebViewDelegate inspectorWKWebViewDidMoveToWindow:self];
+}
+
@end
#endif
Modified: trunk/Source/WebKit/UIProcess/mac/WebInspectorProxyMac.mm (237130 => 237131)
--- trunk/Source/WebKit/UIProcess/mac/WebInspectorProxyMac.mm 2018-10-15 19:14:50 UTC (rev 237130)
+++ trunk/Source/WebKit/UIProcess/mac/WebInspectorProxyMac.mm 2018-10-15 19:28:09 UTC (rev 237131)
@@ -50,6 +50,8 @@
// Reusing the WebView improves start up time for people that jump in and out of the Inspector.
static const Seconds webViewCloseTimeout { 1_min };
+static void* kWindowContentLayoutObserverContext = &kWindowContentLayoutObserverContext;
+
@interface WKWebInspectorProxyObjCAdapter () <WKInspectorViewControllerDelegate>
- (instancetype)initWithWebInspectorProxy:(WebKit::WebInspectorProxy*)inspectorProxy;
@@ -134,6 +136,24 @@
});
}
+- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary<NSKeyValueChangeKey, id> *)change context:(void *)context
+{
+ if (context != kWindowContentLayoutObserverContext) {
+ [super observeValueForKeyPath:keyPath ofObject:object change:change context:context];
+ return;
+ }
+
+ NSWindow *window = object;
+ ASSERT([window isKindOfClass:[NSWindow class]]);
+ if (window.inLiveResize)
+ return;
+
+ dispatch_after(DISPATCH_TIME_NOW, dispatch_get_main_queue(), ^{
+ if (_inspectorProxy)
+ _inspectorProxy->inspectedViewFrameDidChange();
+ });
+}
+
// MARK: WKInspectorViewControllerDelegate methods
- (void)inspectorViewControllerInspectorDidCrash:(WKInspectorViewController *)inspectorViewController
@@ -147,6 +167,18 @@
return _inspectorProxy ? _inspectorProxy->isUnderTest() : false;
}
+- (void)inspectorViewController:(WKInspectorViewController *)inspectorViewController willMoveToWindow:(NSWindow *)newWindow
+{
+ if (_inspectorProxy)
+ _inspectorProxy->attachmentWillMoveFromWindow(inspectorViewController.webView.window);
+}
+
+- (void)inspectorViewControllerDidMoveToWindow:(WKInspectorViewController *)inspectorViewController
+{
+ if (_inspectorProxy)
+ _inspectorProxy->attachmentDidMoveToWindow(inspectorViewController.webView.window);
+}
+
@end
namespace WebKit {
@@ -161,6 +193,23 @@
attach(m_attachmentSide);
}
+void WebInspectorProxy::attachmentWillMoveFromWindow(NSWindow *oldWindow)
+{
+ if (m_isObservingContentLayoutRect) {
+ m_isObservingContentLayoutRect = false;
+ [oldWindow removeObserver:m_objCAdapter.get() forKeyPath:@"contentLayoutRect" context:kWindowContentLayoutObserverContext];
+ }
+}
+
+void WebInspectorProxy::attachmentDidMoveToWindow(NSWindow *newWindow)
+{
+ if (m_isAttached && !!newWindow) {
+ m_isObservingContentLayoutRect = true;
+ [newWindow addObserver:m_objCAdapter.get() forKeyPath:@"contentLayoutRect" options:0 context:kWindowContentLayoutObserverContext];
+ inspectedViewFrameDidChange();
+ }
+}
+
void WebInspectorProxy::setInspectorWindowFrame(WKRect& frame)
{
if (m_isAttached)
@@ -523,14 +572,12 @@
// Preserve the top position of the inspected view so banners in Safari still work. But don't use that
// top position for the inspector view since the banners only stretch as wide as the inspected view.
inspectedViewFrame = NSMakeRect(0, 0, parentWidth - inspectorWidth, inspectedViewTop);
- CGFloat insetExcludingBanners = 0;
- ALLOW_DEPRECATED_DECLARATIONS_BEGIN
- if ([inspectedView isKindOfClass:[WKView class]])
- insetExcludingBanners = ((WKView *)inspectedView)._topContentInset - ((WKView *)inspectedView)._totalHeightOfBanners;
- ALLOW_DEPRECATED_DECLARATIONS_END
- if ([inspectedView isKindOfClass:[WKWebView class]])
- insetExcludingBanners = ((WKWebView *)inspectedView)._topContentInset - ((WKWebView *)inspectedView)._totalHeightOfBanners;
- newInspectorViewFrame = NSMakeRect(parentWidth - inspectorWidth, 0, inspectorWidth, NSHeight(parentBounds) - insetExcludingBanners);
+ newInspectorViewFrame = NSMakeRect(parentWidth - inspectorWidth, 0, inspectorWidth, NSHeight(parentBounds));
+
+ if (NSWindow *inspectorWindow = inspectorView.window) {
+ NSRect contentLayoutRect = [inspectedView.superview convertRect:inspectorWindow.contentLayoutRect fromView:nil];
+ newInspectorViewFrame = NSIntersectionRect(newInspectorViewFrame, contentLayoutRect);
+ }
break;
}
@@ -544,14 +591,12 @@
// Preserve the top position of the inspected view so banners in Safari still work. But don't use that
// top position for the inspector view since the banners only stretch as wide as the inspected view.
inspectedViewFrame = NSMakeRect(inspectorWidth, 0, parentWidth - inspectorWidth, inspectedViewTop);
- CGFloat insetExcludingBanners = 0;
- ALLOW_DEPRECATED_DECLARATIONS_BEGIN
- if ([inspectedView isKindOfClass:[WKView class]])
- insetExcludingBanners = ((WKView *)inspectedView)._topContentInset - ((WKView *)inspectedView)._totalHeightOfBanners;
- ALLOW_DEPRECATED_DECLARATIONS_END
- if ([inspectedView isKindOfClass:[WKWebView class]])
- insetExcludingBanners = ((WKWebView *)inspectedView)._topContentInset - ((WKWebView *)inspectedView)._totalHeightOfBanners;
- newInspectorViewFrame = NSMakeRect(0, 0, inspectorWidth, NSHeight(parentBounds) - insetExcludingBanners);
+ newInspectorViewFrame = NSMakeRect(0, 0, inspectorWidth, NSHeight(parentBounds));
+
+ if (NSWindow *inspectorWindow = inspectorView.window) {
+ NSRect contentLayoutRect = [inspectedView.superview convertRect:inspectorWindow.contentLayoutRect fromView:nil];
+ newInspectorViewFrame = NSIntersectionRect(newInspectorViewFrame, contentLayoutRect);
+ }
break;
}
}
