Bug 192644

Summary: WKWebView returns invalid window screen width and height in classic iPad apps
Product: WebKit Reporter: Andrew Wooster <andrew>
Component: WebCore JavaScriptAssignee: Nobody <webkit-unassigned>
Status: NEW    
Severity: Normal CC: achristensen, aestes, dbates, ews-watchlist, jbedard, thorton, wenson_hsieh
Priority: P2    
Version: WebKit Local Build   
Hardware: iPhone / iPad   
OS: iOS 12   
Attachments:
Description Flags
Sample iPad app that runs in classic mode
none
Patch
none
Patch none

Andrew Wooster
Reported 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.
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
DetailsFormatted DiffDiff
Patch (1.53 KB, patch)
2018-12-13 15:34 PST, Andrew Wooster 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Andrew Wooster
Comment 1 2018-12-12 17:57:50 PST
Created attachment 357200 [details] Patch
EWS Watchlist
Comment 2 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.
Andrew Wooster
Comment 3 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>
Daniel Bates
Comment 4 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?
Andrew Wooster
Comment 5 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.
Andrew Wooster
Comment 6 2018-12-13 15:34:52 PST
Created attachment 357264 [details] Patch
Andrew Wooster
Comment 7 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.
Jonathan Bedard
Comment 8 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>.
Alex Christensen
Comment 9 2021-11-01 12:44:09 PDT
Comment on attachment 357264 [details] Patch This has been requesting review for more than one year. If this is still needed, please rebase and re-request review.
Note You need to log in before you can comment on or make changes to this bug.