Bug 194892 - scalableNativeWebpageParameters() is not preserved on new page navigation.
Summary: scalableNativeWebpageParameters() is not preserved on new page navigation.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yongjun Zhang
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-02-20 19:41 PST by Yongjun Zhang
Modified: 2019-02-25 20:39 PST (History)
9 users (show)

See Also:

Attachments
Patch (16.43 KB, patch)
2019-02-20 20:55 PST, Yongjun Zhang 		no flags Details | Formatted Diff | Diff
Fix Mac build break. (16.43 KB, patch)
2019-02-20 22:32 PST, Yongjun Zhang 		ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews124 for ios-simulator-wk2 (2.57 MB, application/zip)
2019-02-21 00:24 PST, EWS Watchlist 		no flags Details
Fix DRT build break. (18.93 KB, patch)
2019-02-21 10:46 PST, Yongjun Zhang 		ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews125 for ios-simulator-wk2 (2.48 MB, application/zip)
2019-02-21 12:47 PST, EWS Watchlist 		no flags Details
Fix a failed layout test. (16.63 KB, patch)
2019-02-21 14:05 PST, Yongjun Zhang 		ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews124 for ios-simulator-wk2 (2.54 MB, application/zip)
2019-02-21 16:39 PST, EWS Watchlist 		no flags Details
Fix a failed layout test again. (17.67 KB, patch)
2019-02-22 15:06 PST, Yongjun Zhang 		wenson_hsieh: review+
Details | Formatted Diff | Diff
Address review comments and commit. (18.06 KB, patch)
2019-02-25 19:58 PST, Yongjun Zhang 		no flags Details | Formatted Diff | Diff
Show Obsolete (8) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yongjun Zhang 2019-02-20 19:41:05 PST 
If a page's current default viewport configuration is scalableNativeWebpageParameters, loading a new page should preserve this configuration.
Comment 1 Yongjun Zhang 2019-02-20 19:44:35 PST 
rdar://problem/47538280
Comment 2 Yongjun Zhang 2019-02-20 20:55:42 PST 
Created attachment 362585 [details]
Patch
Comment 3 Yongjun Zhang 2019-02-20 22:32:09 PST 
Created attachment 362594 [details]
Fix Mac build break.
Comment 4 EWS Watchlist 2019-02-21 00:24:04 PST 
Comment on attachment 362594 [details]
Fix Mac build break.

Attachment 362594 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11229089

New failing tests:
fast/viewport/ios/minimum-scale-after-changing-view-scale.html
Comment 5 EWS Watchlist 2019-02-21 00:24:06 PST 
Created attachment 362598 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 6 Yongjun Zhang 2019-02-21 10:46:38 PST 
Created attachment 362619 [details]
Fix DRT build break.
Comment 7 EWS Watchlist 2019-02-21 12:47:25 PST 
Comment on attachment 362619 [details]
Fix DRT build break.

Attachment 362619 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11234456

New failing tests:
fast/viewport/ios/preserve-viewport-shrink-to-fit-on-new-navigation.html
fast/viewport/ios/minimum-scale-after-changing-view-scale.html
Comment 8 EWS Watchlist 2019-02-21 12:47:27 PST 
Created attachment 362630 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 9 Yongjun Zhang 2019-02-21 14:05:00 PST 
Created attachment 362642 [details]
Fix a failed layout test.
Comment 10 EWS Watchlist 2019-02-21 16:39:19 PST 
Comment on attachment 362642 [details]
Fix a failed layout test.

Attachment 362642 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11237861

New failing tests:
fast/viewport/ios/minimum-scale-after-changing-view-scale.html
Comment 11 EWS Watchlist 2019-02-21 16:39:21 PST 
Created attachment 362670 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 12 Yongjun Zhang 2019-02-22 15:06:34 PST 
Created attachment 362768 [details]
Fix a failed layout test again.
Comment 13 Wenson Hsieh 2019-02-25 14:53:18 PST 
Comment on attachment 362768 [details]
Fix a failed layout test again.

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

> Source/WebCore/page/ViewportConfiguration.cpp:336
> +ViewportConfiguration::Parameters ViewportConfiguration::fixedNativeWebpageParameters()

It's somewhat confusing that nativeWebpageParameters splits off into scalableNativeWebpageParameters or fixedNativeWebpageParameters. I think fixedNativeWebpageParameters is a bit of a strange name too (the key point here is that the fixed parameters don't allow shrink to fit, right?)

Maybe we can make this more explicit, like...

nativeWebpageParameters() returns either nativeWebpageParametersWithShrinkToFit() or nativeWebpageParametersWithoutShrinkToFit()?

> Tools/WebKitTestRunner/UIScriptControllerCocoa.mm:60
> +    TestController::singleton().mainWebView()->platformView()._allowsViewportShrinkToFit = allows;

Nit - I think you could just put this in UIScriptControllerIOS.mm, since _allowsViewportShrinkToFit is only available on iOS.
Comment 14 Yongjun Zhang 2019-02-25 17:06:32 PST 
(In reply to Wenson Hsieh from comment #13)
> Comment on attachment 362768 [details]
> Fix a failed layout test again.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=362768&action=review
> 
> > Source/WebCore/page/ViewportConfiguration.cpp:336
> > +ViewportConfiguration::Parameters ViewportConfiguration::fixedNativeWebpageParameters()
> 
> It's somewhat confusing that nativeWebpageParameters splits off into
> scalableNativeWebpageParameters or fixedNativeWebpageParameters. I think
> fixedNativeWebpageParameters is a bit of a strange name too (the key point
> here is that the fixed parameters don't allow shrink to fit, right?)
> 
> Maybe we can make this more explicit, like...
> 
> nativeWebpageParameters() returns either
> nativeWebpageParametersWithShrinkToFit() or
> nativeWebpageParametersWithoutShrinkToFit()?
> 

Good point! Will change.

> > Tools/WebKitTestRunner/UIScriptControllerCocoa.mm:60
> > +    TestController::singleton().mainWebView()->platformView()._allowsViewportShrinkToFit = allows;
> 
> Nit - I think you could just put this in UIScriptControllerIOS.mm, since
> _allowsViewportShrinkToFit is only available on iOS.
Comment 15 Yongjun Zhang 2019-02-25 19:58:06 PST 
Created attachment 362953 [details]
Address review comments and commit.
Comment 16 WebKit Commit Bot 2019-02-25 20:38:05 PST 
Comment on attachment 362953 [details]
Address review comments and commit.

Clearing flags on attachment: 362953

Committed r242069: <https://trac.webkit.org/changeset/242069>
Comment 17 WebKit Commit Bot 2019-02-25 20:38:06 PST 
All reviewed patches have been landed.  Closing bug.
Comment 18 Radar WebKit Bug Importer 2019-02-25 20:39:31 PST 
<rdar://problem/48389409>