Bug 192644 - WKWebView returns invalid window screen width and height in classic iPad apps
Summary: WKWebView returns invalid window screen width and height in classic iPad apps
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: WebKit Local Build
Hardware: iPhone / iPad iOS 12
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-12-12 16:14 PST by Andrew Wooster
Modified: 2018-12-14 12:49 PST (History)
6 users (show)

See Also:


Attachments
Sample iPad app that runs in classic mode (57.50 KB, application/zip)
2018-12-12 16:14 PST, Andrew Wooster
no flags Details
Patch (1.48 KB, patch)
2018-12-12 17:57 PST, Andrew Wooster
no flags Details | Formatted Diff | Diff
Patch (1.53 KB, patch)
2018-12-13 15:34 PST, Andrew Wooster
andrew: review?
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Wooster 2018-12-12 16:14:36 PST
Created attachment 357186 [details]
Sample iPad app that runs in classic mode

Overview:

WKWebView returns incorrect values for window.screen.width and window.screen.height in JavaScript when inside iPad apps running in a scaled/non-native mode ("classic"). This is a regression from UIWebView behavior.

Steps to Reproduce:

* Open attached sample app on a 10.5", 11", or 12.9" iPad or iPad simulator.
* Note difference between values in the top WKWebView versus the bottom UIWebView.

Actual Results:

In the WKWebView window.screen.width is 320 and window.screen.height is 480.
In the UIWebView window.screen.width is 768 and window.screen.height is 1024.

Expected Results:

The screen size should report as 768x1024, which is the screen's reference bounds.

Configuration:

Found on iOS 12.1 both in Simulator and on device, although haven't regressed it. Xcode 10.1 (10B61).

Additional Notes:

320x480 is the default scaling resolution for iPhone-only apps whether on iPhones or iPads. 768x1024 is the scaling resolution of iPad apps on iPads, as that was the resolution of the first iPad.

This looks like it was a deliberate change in https://bugs.webkit.org/show_bug.cgi?id=150763, but I don't have any insight as to why this change was made.

This breaks some existing functionality when migrating from UIWebView to WKWebView in scaled iPad apps. Workaround is to support the native resolution of each iPad, but in legacy iPad apps that process may take longer than moving to WKWebView.
Comment 1 Andrew Wooster 2018-12-12 17:57:50 PST
Created attachment 357200 [details]
Patch
Comment 2 Build Bot 2018-12-12 18:04:39 PST
Attachment 357200 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 1 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Andrew Wooster 2018-12-12 19:42:34 PST
Per the style failure, it doesn't look like there's a mechanism in the test harnesses to test the iOS classic mode on various iOS devices. If that's not the case, let me know and I can try to add tests for this.

Also, I'm having some difficulty getting run-webkit-tests to run due to compilation errors in LayoutTestHelper along the lines of:

> In file included from …WebKit/Tools/DumpRenderTree/mac/LayoutTestHelper.m:32:
> …WebKit/Tools/DumpRenderTree/config.h:26:10: fatal error: 'WebCore/PlatformExportMacros.h' file not found
> #include <WebCore/PlatformExportMacros.h>
Comment 4 Daniel Bates 2018-12-12 20:17:33 PST
(In reply to Andrew Wooster from comment #3)
> Per the style failure, it doesn't look like there's a mechanism in the test
> harnesses to test the iOS classic mode on various iOS devices. If that's not
> the case, let me know and I can try to add tests for this.
> 
> Also, I'm having some difficulty getting run-webkit-tests to run due to
> compilation errors in LayoutTestHelper along the lines of:
> 
> > In file included from …WebKit/Tools/DumpRenderTree/mac/LayoutTestHelper.m:32:
> > …WebKit/Tools/DumpRenderTree/config.h:26:10: fatal error: 'WebCore/PlatformExportMacros.h' file not found
> > #include <WebCore/PlatformExportMacros.h>

Are you passing —ios-simulator (starts with two dashes) to run-webkit-tests?
Comment 5 Andrew Wooster 2018-12-12 20:31:25 PST
Oops, nope, that works. Looks like there are some failures in fast/viewport/ios/ipad that I'll look into tomorrow.
Comment 6 Andrew Wooster 2018-12-13 15:34:52 PST
Created attachment 357264 [details]
Patch
Comment 7 Andrew Wooster 2018-12-13 15:39:11 PST
Updated the patch with a note about tests.

The test failures I saw appeared on both WebKit with and without the patch. They show up as "Flaky tests".

Additionally, it looks like not many tests are being run by the test runner:

> Found 56768 tests; running 48958, skipping 7810.
>                                      
> Skipping 48945 tests because iPhone SE running iOS is not available
> 
> 
> Skipping 1 test because iPhone 7 running iOS is not available
> 
> 
> Running 12 tests for iPad running iOS

I'm running Xcode 10.1.
Comment 8 Jonathan Bedard 2018-12-14 11:18:37 PST
(In reply to Andrew Wooster from comment #7)
> Updated the patch with a note about tests.
> 
> The test failures I saw appeared on both WebKit with and without the patch.
> They show up as "Flaky tests".
> 
> Additionally, it looks like not many tests are being run by the test runner:
> 
> > Found 56768 tests; running 48958, skipping 7810.
> >                                      
> > Skipping 48945 tests because iPhone SE running iOS is not available
> > 
> > 
> > Skipping 1 test because iPhone 7 running iOS is not available
> > 
> > 
> > Running 12 tests for iPad running iOS
> 
> I'm running Xcode 10.1.

This is related to some new changes with how we handle already booted simulators. The log you have provided is a bit sparse, (use --debug-rwt-logging to make it more verbose), but I'm betting that you have an iPad (and only an iPad) simulator booted when you run tests. Booting simulators is a bit of an expensive task, so we try to use existing simulators. With some recent changes I've made, we're trying to do a better job of respecting the type of device a test was originally baselined for. You can override the behavior by a) shutting down all simulators before running tests or b) pass --dedicated-simulators.

The relevant bugs (one landed, one still a work in progress) are <https://bugs.webkit.org/show_bug.cgi?id=192161> and <https://bugs.webkit.org/show_bug.cgi?id=192162>.