Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Viewport layout size is sometimes incorrect when a site specifies an initial scale #14566

Merged
merged 1 commit into from Jun 3, 2023

Conversation

rr-codes
Copy link
Contributor

@rr-codes rr-codes commented Jun 1, 2023

3cfe137

Viewport layout size is sometimes incorrect when a site specifies an initial scale
https://bugs.webkit.org/show_bug.cgi?id=257583
rdar://109483331

Reviewed by Dean Jackson.

On iOS, if a page specifies a viewport meta tag including an initial scale value that is < 1,
and if the `minimumEffectiveDeviceWidth` is non-zero, the resulting layout width will have an
undesirable value. Instead, the initial scale should be ignored when the `minimumEffectiveDeviceWidth`
is specified when computing the layout width.

This PR fixes this by ignoring the initial scale only when the `minimumEffectiveDeviceWidth` actually
has an effect on the viewport layout width.

Also refactors the `applyViewportArgument` to be simpler.

* Source/WebCore/page/ViewportConfiguration.cpp:
(WebCore::applyViewportArgument):
(WebCore::ViewportConfiguration::updateConfiguration):
(WebCore::ViewportConfiguration::layoutWidth const):
* Source/WebCore/page/ViewportConfiguration.h:
(WebCore::ViewportConfiguration::Parameters::operator== const):
* Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* Tools/TestWebKitAPI/Tests/ios/Viewport.mm: Added.
(TestWebKitAPI::TEST):

Canonical link: https://commits.webkit.org/264842@main

52695d3

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl ❌ πŸ§ͺ ios-wk2   πŸ§ͺ api-mac βœ… πŸ›  gtk
βœ… πŸ§ͺ ios-wk2-wpt βœ… πŸ§ͺ mac-wk1 βœ… πŸ§ͺ gtk-wk2
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 βœ… πŸ§ͺ api-gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  tv-sim
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch
βœ… πŸ›  watch-sim

@rr-codes rr-codes requested a review from cdumez as a code owner June 1, 2023 02:40
@rr-codes rr-codes self-assigned this Jun 1, 2023
@rr-codes rr-codes added the Layout and Rendering For bugs with layout and rendering of Web pages. label Jun 1, 2023
@smfr
Copy link
Contributor

smfr commented Jun 1, 2023

Can we make a layout test or API test for this?

Copy link
Contributor

@grorg grorg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me but I agree with Simon that there should be a test.

@rr-codes
Copy link
Contributor Author

rr-codes commented Jun 1, 2023

r=me but I agree with Simon that there should be a test.

Yup, working on an API test rn

Copy link
Member

@aprotyas aprotyas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-reviewer r+

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 2, 2023
@rr-codes rr-codes added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels Jun 3, 2023
…initial scale

https://bugs.webkit.org/show_bug.cgi?id=257583
rdar://109483331

Reviewed by Dean Jackson.

On iOS, if a page specifies a viewport meta tag including an initial scale value that is < 1,
and if the `minimumEffectiveDeviceWidth` is non-zero, the resulting layout width will have an
undesirable value. Instead, the initial scale should be ignored when the `minimumEffectiveDeviceWidth`
is specified when computing the layout width.

This PR fixes this by ignoring the initial scale only when the `minimumEffectiveDeviceWidth` actually
has an effect on the viewport layout width.

Also refactors the `applyViewportArgument` to be simpler.

* Source/WebCore/page/ViewportConfiguration.cpp:
(WebCore::applyViewportArgument):
(WebCore::ViewportConfiguration::updateConfiguration):
(WebCore::ViewportConfiguration::layoutWidth const):
* Source/WebCore/page/ViewportConfiguration.h:
(WebCore::ViewportConfiguration::Parameters::operator== const):
* Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* Tools/TestWebKitAPI/Tests/ios/Viewport.mm: Added.
(TestWebKitAPI::TEST):

Canonical link: https://commits.webkit.org/264842@main
@webkit-commit-queue
Copy link
Collaborator

Committed 264842@main (3cfe137): https://commits.webkit.org/264842@main

Reviewed commits have been landed. Closing PR #14566 and removing active labels.

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jun 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Layout and Rendering For bugs with layout and rendering of Web pages.
Projects
None yet
7 participants