Ads in NYTimes app are shifted downwards by the scroll view's top content inset
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 3 May 2018 20:16:14 +0000 (20:16 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 3 May 2018 20:16:14 +0000 (20:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=185251
<rdar://problem/39062357>

Reviewed by Tim Horton.

Source/WebKit:

The NYTimes app embeds advertisements in each article's WKWebView by adding views in the WKScrollView's view
hierarchy. These views are positioned using the bounding client rects of elements in the DOM (via Element
::getBoundingClientRect). Prior to r229641, WebKit would report bounding client rects inset by the content
insets of WKScrollView, which means that if a top content inset X is specified on the scroll view, an element
that is flush against the top of the viewport will have a bounding client rect top of -X (when it should really
be 0).

To account for this, NYTimes adds the scroll view content insets back to the bounding client rect when
determining the position of each advertisement which, after r229641, causes these views to be shifted downwards
by an amount equal to the scroll view content inset top.

This new behavior does not affect Safari, since Safari uses SPI to explicitly set obscured insets. As such, we
address this by gating the scroll view content inset fix with a linked-on-or-after check.

* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _computedObscuredInset]):
* UIProcess/Cocoa/VersionChecks.h:

Source/WTF:

Add a new DYLD_IOS_VERSION macro definition for previous or non-internal SDKs.

* wtf/spi/darwin/dyldSPI.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@231322 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WTF/ChangeLog
Source/WTF/wtf/spi/darwin/dyldSPI.h
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
Source/WebKit/UIProcess/Cocoa/VersionChecks.h

index 85bbd26..9e176b3 100644 (file)
@@ -1,3 +1,15 @@
+2018-05-03  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Ads in NYTimes app are shifted downwards by the scroll view's top content inset
+        https://bugs.webkit.org/show_bug.cgi?id=185251
+        <rdar://problem/39062357>
+
+        Reviewed by Tim Horton.
+
+        Add a new DYLD_IOS_VERSION macro definition for previous or non-internal SDKs.
+
+        * wtf/spi/darwin/dyldSPI.h:
+
 2018-05-03  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r231197.
index 56efeff..4506098 100644 (file)
 #define DYLD_IOS_VERSION_11_3 0x000B0300
 #endif
 
+#ifndef DYLD_IOS_VERSION_12_0
+#define DYLD_IOS_VERSION_12_0 0x000C0000
+#endif
+
 #ifndef DYLD_MACOSX_VERSION_10_13
 #define DYLD_MACOSX_VERSION_10_13 0x000A0D00
 #endif
@@ -52,6 +56,7 @@
 #define DYLD_IOS_VERSION_10_0 0x000A0000
 #define DYLD_IOS_VERSION_11_0 0x000B0000
 #define DYLD_IOS_VERSION_11_3 0x000B0300
+#define DYLD_IOS_VERSION_12_0 0x000C0000
 
 #define DYLD_MACOSX_VERSION_10_11 0x000A0B00
 #define DYLD_MACOSX_VERSION_10_12 0x000A0C00
index bb63213..4720a83 100644 (file)
@@ -1,3 +1,29 @@
+2018-05-03  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Ads in NYTimes app are shifted downwards by the scroll view's top content inset
+        https://bugs.webkit.org/show_bug.cgi?id=185251
+        <rdar://problem/39062357>
+
+        Reviewed by Tim Horton.
+
+        The NYTimes app embeds advertisements in each article's WKWebView by adding views in the WKScrollView's view
+        hierarchy. These views are positioned using the bounding client rects of elements in the DOM (via Element
+        ::getBoundingClientRect). Prior to r229641, WebKit would report bounding client rects inset by the content
+        insets of WKScrollView, which means that if a top content inset X is specified on the scroll view, an element
+        that is flush against the top of the viewport will have a bounding client rect top of -X (when it should really
+        be 0).
+
+        To account for this, NYTimes adds the scroll view content insets back to the bounding client rect when
+        determining the position of each advertisement which, after r229641, causes these views to be shifted downwards
+        by an amount equal to the scroll view content inset top.
+
+        This new behavior does not affect Safari, since Safari uses SPI to explicitly set obscured insets. As such, we
+        address this by gating the scroll view content inset fix with a linked-on-or-after check.
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _computedObscuredInset]):
+        * UIProcess/Cocoa/VersionChecks.h:
+
 2018-05-03  Chris Dumez  <cdumez@apple.com>
 
         Load hangs if the WebProcess fails to launch
index 86c8362..235a718 100644 (file)
@@ -1617,6 +1617,12 @@ static WebCore::Color scrollViewBackgroundColor(WKWebView *webView)
 
 - (UIEdgeInsets)_computedObscuredInset
 {
+    if (!linkedOnOrAfter(WebKit::SDKVersion::FirstWhereScrollViewContentInsetsAreNotObscuringInsets)) {
+        // For binary compability with third party apps, treat scroll view content insets as obscuring insets when the app is compiled
+        // against a WebKit version without the fix in r229641.
+        return [self _computedContentInset];
+    }
+
     if (_haveSetObscuredInsets)
         return _obscuredInsets;
 
index efa4155..8ba324a 100644 (file)
@@ -38,6 +38,7 @@ enum class SDKVersion : uint32_t {
     FirstWithExpiredOnlyReloadBehavior = DYLD_IOS_VERSION_11_0,
     FirstThatDisallowsSettingAnyXHRHeaderFromFileURLs = DYLD_IOS_VERSION_11_3,
     FirstThatDefaultsToPassiveTouchListenersOnDocument = DYLD_IOS_VERSION_11_3,
+    FirstWhereScrollViewContentInsetsAreNotObscuringInsets = DYLD_IOS_VERSION_12_0,
 #elif PLATFORM(MAC)
     FirstWithNetworkCache = DYLD_MACOSX_VERSION_10_11,
     FirstWithExceptionsForDuplicateCompletionHandlerCalls = DYLD_MACOSX_VERSION_10_13,