Title: [231322] trunk/Source
Revision
231322
Author
[email protected]
Date
2018-05-03 13:16:14 -0700 (Thu, 03 May 2018)

Log Message

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.

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:

Modified Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (231321 => 231322)


--- trunk/Source/WTF/ChangeLog	2018-05-03 19:59:05 UTC (rev 231321)
+++ trunk/Source/WTF/ChangeLog	2018-05-03 20:16:14 UTC (rev 231322)
@@ -1,3 +1,15 @@
+2018-05-03  Wenson Hsieh  <[email protected]>
+
+        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  <[email protected]>
 
         Unreviewed, rolling out r231197.

Modified: trunk/Source/WTF/wtf/spi/darwin/dyldSPI.h (231321 => 231322)


--- trunk/Source/WTF/wtf/spi/darwin/dyldSPI.h	2018-05-03 19:59:05 UTC (rev 231321)
+++ trunk/Source/WTF/wtf/spi/darwin/dyldSPI.h	2018-05-03 20:16:14 UTC (rev 231322)
@@ -37,6 +37,10 @@
 #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

Modified: trunk/Source/WebKit/ChangeLog (231321 => 231322)


--- trunk/Source/WebKit/ChangeLog	2018-05-03 19:59:05 UTC (rev 231321)
+++ trunk/Source/WebKit/ChangeLog	2018-05-03 20:16:14 UTC (rev 231322)
@@ -1,3 +1,29 @@
+2018-05-03  Wenson Hsieh  <[email protected]>
+
+        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  <[email protected]>
 
         Load hangs if the WebProcess fails to launch

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm (231321 => 231322)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm	2018-05-03 19:59:05 UTC (rev 231321)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm	2018-05-03 20:16:14 UTC (rev 231322)
@@ -1617,6 +1617,12 @@
 
 - (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;
 

Modified: trunk/Source/WebKit/UIProcess/Cocoa/VersionChecks.h (231321 => 231322)


--- trunk/Source/WebKit/UIProcess/Cocoa/VersionChecks.h	2018-05-03 19:59:05 UTC (rev 231321)
+++ trunk/Source/WebKit/UIProcess/Cocoa/VersionChecks.h	2018-05-03 20:16:14 UTC (rev 231322)
@@ -38,6 +38,7 @@
     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,
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to