Bug 200003 - New pages don't account for device orientation.
Summary: New pages don't account for device orientation.
Status: RESOLVED DUPLICATE of bug 200624
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: Safari 12
Hardware: iPhone / iPad All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-07-22 10:03 PDT by Alfred
Modified: 2022-02-10 16:54 PST (History)
8 users (show)

See Also:


Attachments
Save viewport size in dynamicViewportSizeUpdate (2.06 KB, patch)
2019-07-22 11:05 PDT, Alfred
no flags Details | Formatted Diff | Diff
Save viewport size in dynamicViewportSizeUpdate (2.10 KB, patch)
2019-07-22 11:42 PDT, Alfred
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alfred 2019-07-22 10:03:25 PDT
Steps to reproduce:
1. Open Safari in portrait and add bing.com(1) to favorites.
2. Open any site (for example apple.com)
3. Turn device to landscape
4. Press on omnibox and open bing.com(1) from favorites.
Result: Page renders like it is in portrait, leaving most of the screen black.
Expected result: Page renders in lanscape orientation filling all available width.

Note: 
(1) It is not unique to bing.com, but some (like apple.com) relayout just after loading, and some (like google.com or yahoo.com) relayout after some scroll.
(3) Bug reproduces also when switching from landscape to portrait.
(2) It is important, that site will be loaded from favorites, because typing address to omnibox will trigger creation of another WKWebView and it will catch up correct values.
Comment 1 Radar WebKit Bug Importer 2019-07-22 10:08:10 PDT
<rdar://problem/53403006>
Comment 2 Alfred 2019-07-22 11:05:49 PDT
Created attachment 374607 [details]
Save viewport size in dynamicViewportSizeUpdate
Comment 3 EWS Watchlist 2019-07-22 11:15:36 PDT
Attachment 374607 [details] did not pass style-queue:


ERROR: Source/WebKit/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/WebKit/ChangeLog:10:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/WebKit/ChangeLog:11:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/WebKit/ChangeLog:12:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/WebKit/ChangeLog:13:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/WebKit/ChangeLog:14:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 6 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Alfred 2019-07-22 11:42:37 PDT
Created attachment 374616 [details]
Save viewport size in dynamicViewportSizeUpdate
Comment 5 Tim Horton 2019-07-22 11:59:28 PDT
Comment on attachment 374607 [details]
Save viewport size in dynamicViewportSizeUpdate

View in context: https://bugs.webkit.org/attachment.cgi?id=374607&action=review

> Source/WebKit/ChangeLog:3
> +        New pages don't account for device orientation.

"deviceOrientation" is one of the /other/ parameters to dynamicViewportSizeUpdate, so this title is quite confusing; this is really about the layout size

> Source/WebKit/ChangeLog:13
> +	Moreover, in WKWebView there is lastSentViewLayoutSize property, which is used to not update viewport size
> +	if it is not changed. So, it is blocking new updates to pass to 

_lastSentViewLayoutSize is reset upon process swap or crash, so I don't quite get this part

> Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm:298
> +    m_viewportConfigurationViewLayoutSize = viewLayoutSize;

It seems surprising that we'd only need to save one of the parameters (what happens to e.g. maximumUnobscuredSize?). I wonder if it's missing from elsewhere instead of needing to be added here?
Comment 6 Alfred 2019-07-22 13:40:11 PDT
Comment on attachment 374607 [details]
Save viewport size in dynamicViewportSizeUpdate

View in context: https://bugs.webkit.org/attachment.cgi?id=374607&action=review

>> Source/WebKit/ChangeLog:3
>> +        New pages don't account for device orientation.
> 
> "deviceOrientation" is one of the /other/ parameters to dynamicViewportSizeUpdate, so this title is quite confusing; this is really about the layout size

Yes, you are right, it is confusing from implementation point of view, but from user's perspective this is what actually happens. 
I tried to name the bug from the user's perspective, but I didn't know that bug title should be included in changelog file. Do you think that I should rename title to something like "New pages don't account for layout size after rotation"?

>> Source/WebKit/ChangeLog:13
>> +	if it is not changed. So, it is blocking new updates to pass to 
> 
> _lastSentViewLayoutSize is reset upon process swap or crash, so I don't quite get this part

(1) One of the possible workarounds is to trigger layout size update just after rotation via `_overrideLayoutParametersWithMinimumLayoutSize:maximumUnobscuredSizeOverride:` -> `_setViewLayoutSizeOverride` -> `_dispatchSetViewLayoutSize` -> `setViewportConfigurationViewLayoutSize`, but it early exits in `_dispatchSetViewLayoutSize` because `_lastSentViewLayoutSize` didn't change. Of course, it will reset when actual process swap occurs, but new page will be created with wrong values at this point.
(2) In `_didCompleteAnimatedResize` there is call to `_dispatchSetViewLayoutSize`, but it wrapped in `newViewLayoutSize != _lastSentViewLayoutSize.value()` condition.

>> Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm:298
>> +    m_viewportConfigurationViewLayoutSize = viewLayoutSize;
> 
> It seems surprising that we'd only need to save one of the parameters (what happens to e.g. maximumUnobscuredSize?). I wonder if it's missing from elsewhere instead of needing to be added here?

Firstly I tried to save `maximumUnobscuredSize` also, but I couldn't observe any effect off this. Unfortunately, I din't figure out a way how to initialize new page with correct knowledge about unobscured size/obscured insets yet, so now it initializes with default zero values (You can observe this by rotating to landscape and navigating to webpage from favorites on iPhone X).
Comment 7 Alfred 2020-01-10 09:14:54 PST
I think that was fixed in https://bugs.webkit.org/show_bug.cgi?id=200624

*** This bug has been marked as a duplicate of bug 200624 ***