Skip to content

Commit

Permalink
[iPadOS] Google search results page may be clipped in portrait mode w…
Browse files Browse the repository at this point in the history
…ith stage manager enabled

https://bugs.webkit.org/show_bug.cgi?id=260773
rdar://109158566

Reviewed by Tim Horton.

When stage manager is enabled (i.e. `-[UIWindowScene _enhancedWindowingEnabled]` is `YES`) and we're
loading desktop-class websites with no meta viewport, our viewport configuration strategy is:

1.  If the web view is wider than 980 pt, shrink to fit the view.
2.  If the web view is narrower than 980 pt, shrink to fit 980 pt, and then let the rest of the page
    overflow horizontally. This was devised as a way to avoid extremely awkward viewport sizing
    behaviors when resizing windows on iPad to very narrow widths in stage manager, since the entire
    page would be scaled down so much, that it would be left with no legible text.

To achieve (2), 250431@main adjusted some logic when shrinking the viewport to fit, so that we'd
shrink down to fit 980 pt instead of the actual view width when computing the initial scale, when
the stage manager window is smaller than 980 pt. However, this means that on some models of iPad
where portrait mode is as narrow as 820 pt, we get horizontal scrolling in portrait mode, even when
Safari is fullscreen. At the same time, shrinking down to 980 pt also breaks layout on Google search
results, due to (what appears to be) an issue with the site itself when determining which search
results layout to use.

To mitigate both the horizontal scrolling and the site issue on Google search results for now,
simply adjust the (arbitrarily chosen) desktop viewport width of 980 to 820, which is the width of
the narrowest iPad in portrait mode that currently supports Stage Manager, iPad Air (5th gen).

Test: fast/viewport/ios/wide-desktop-viewport-in-enhanced-windowing-mode.html

* LayoutTests/fast/viewport/ios/wide-desktop-viewport-in-enhanced-windowing-mode-expected.txt: Added.
* LayoutTests/fast/viewport/ios/wide-desktop-viewport-in-enhanced-windowing-mode.html: Added.

Add a layout test that enables stage manager and desktop-class website viewport behaviors, to verify
that we shrink a very wide webpage (1280pt) down to 820, and let the remainder overflow
horizontally. Note that this test runs and yields the same results on both iPhone and iPad, since
it doesn't matter what the actual viewport length is — as long as it's below 820pt. Running this on
iPhone effectively simulates changing the stage manager window width on iPad to the narrowest
possible setting, and verifying that it scrolls horizontally instead of scaling down to comically
small proportions.

* Source/WebCore/page/ViewportConfiguration.cpp:
(WebCore::ViewportConfiguration::initialScaleFromSize const):
* Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm:
(-[WKWebView _isWindowResizingEnabled]):

Drive-by fix: remove a runtime selector check that's no longer required.

* Tools/WebKitTestRunner/TestController.h:
* Tools/WebKitTestRunner/TestOptions.cpp:
(WTR::TestOptions::defaults):
(WTR::TestOptions::keyTypeMapping):
* Tools/WebKitTestRunner/TestOptions.h:
(WTR::TestOptions::enhancedWindowingEnabled const):

Add a mechanism to simulate stage manager being enabled, by swizzling `-_enhancedWindowingEnabled`
and returning `YES`.

* Tools/WebKitTestRunner/ios/TestControllerIOS.mm:
(overrideEnhancedWindowingEnabled):
(WTR::TestController::platformResetStateToConsistentValues):

Canonical link: https://commits.webkit.org/267365@main
  • Loading branch information
whsieh committed Aug 28, 2023
1 parent 33b030f commit 85d3ed8
Show file tree
Hide file tree
Showing 8 changed files with 73 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
PASS contentWidth is 1280
PASS contentWidth * scale is 820
PASS successfullyParsed is true

TEST COMPLETE

Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true enhancedWindowingEnabled=true ShouldIgnoreMetaViewport=true ] -->
<html>
<head>
<script src="../../../resources/js-test.js"></script>
<style>
body, html {
margin: 0;
width: 100%;
height: 100%;
}

.horizontal-bar {
background: linear-gradient(to right, red 0%, green 50%, blue 100%);
width: 1280px;
height: 100px;
}
</style>
<script>
jsTestIsAsync = true;
addEventListener("load", async () => {
scale = visualViewport.scale;
contentWidth = document.scrollingElement.scrollWidth;
// The expected value of 820 here matches the minimum shrink-to-fit width when preferring
// horizontal scrolling.
shouldBe("contentWidth", "1280");
shouldBe("contentWidth * scale", "820");
finishJSTest();
});
</script>
</head>
<body>
<div class="horizontal-bar"></div>
</body>
</html>
3 changes: 2 additions & 1 deletion Source/WebCore/page/ViewportConfiguration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ static inline void adjustViewportArgumentsToAvoidExcessiveZooming(ViewportArgume
}

constexpr double defaultDesktopViewportWidth = 980;
constexpr double minimumShrinkToFitWidthWhenPreferringHorizontalScrolling = 820;

#if ASSERT_ENABLED
static bool constraintsAreAllRelative(const ViewportConfiguration::Parameters& configuration)
Expand Down Expand Up @@ -305,7 +306,7 @@ double ViewportConfiguration::initialScaleFromSize(double width, double height,
else if (width > 0) {
auto shrinkToFitWidth = m_viewLayoutSize.width();
if (m_prefersHorizontalScrollingBelowDesktopViewportWidths)
shrinkToFitWidth = std::max<float>(shrinkToFitWidth, std::min(width, defaultDesktopViewportWidth));
shrinkToFitWidth = std::max<float>(shrinkToFitWidth, std::min(width, minimumShrinkToFitWidthWhenPreferringHorizontalScrolling));
initialScale = shrinkToFitWidth / width;
}
}
Expand Down
3 changes: 1 addition & 2 deletions Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1743,8 +1743,7 @@ - (void)didMoveToWindow

- (BOOL)_isWindowResizingEnabled
{
UIWindowScene *scene = self.window.windowScene;
return [scene respondsToSelector:@selector(_enhancedWindowingEnabled)] && scene._enhancedWindowingEnabled;
return self.window.windowScene._enhancedWindowingEnabled;
}

#endif // HAVE(UIKIT_RESIZABLE_WINDOWS)
Expand Down
3 changes: 3 additions & 0 deletions Tools/WebKitTestRunner/TestController.h
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,9 @@ class TestController {
#endif // PLATFORM(COCOA)
#if ENABLE(IMAGE_ANALYSIS)
std::unique_ptr<InstanceMethodSwizzler> m_imageAnalysisRequestSwizzler;
#endif
#if HAVE(UIKIT_RESIZABLE_WINDOWS)
std::unique_ptr<InstanceMethodSwizzler> m_enhancedWindowingEnabledSwizzler;
#endif
bool m_verbose { false };
bool m_printSeparators { false };
Expand Down
2 changes: 2 additions & 0 deletions Tools/WebKitTestRunner/TestOptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ const TestFeatures& TestOptions::defaults()
{ "noUseRemoteLayerTree", false },
{ "useThreadedScrolling", false },
{ "suppressInputAccessoryView", false },
{ "enhancedWindowingEnabled", false },
};
features.doubleTestRunnerFeatures = {
{ "contentInset.top", 0 },
Expand Down Expand Up @@ -247,6 +248,7 @@ const std::unordered_map<std::string, TestHeaderKeyType>& TestOptions::keyTypeMa
{ "useRemoteLayerTree", TestHeaderKeyType::BoolTestRunner },
{ "useThreadedScrolling", TestHeaderKeyType::BoolTestRunner },
{ "suppressInputAccessoryView", TestHeaderKeyType::BoolTestRunner },
{ "enhancedWindowingEnabled", TestHeaderKeyType::BoolTestRunner },

{ "contentInset.top", TestHeaderKeyType::DoubleTestRunner },
{ "obscuredInset.top", TestHeaderKeyType::DoubleTestRunner },
Expand Down
1 change: 1 addition & 0 deletions Tools/WebKitTestRunner/TestOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ class TestOptions {
bool noUseRemoteLayerTree() const { return boolTestRunnerFeatureValue("noUseRemoteLayerTree"); }
bool useThreadedScrolling() const { return boolTestRunnerFeatureValue("useThreadedScrolling"); }
bool suppressInputAccessoryView() const { return boolTestRunnerFeatureValue("suppressInputAccessoryView"); }
bool enhancedWindowingEnabled() const { return boolTestRunnerFeatureValue("enhancedWindowingEnabled"); }
double contentInsetTop() const { return doubleTestRunnerFeatureValue("contentInset.top"); }
double obscuredInsetTop() const { return doubleTestRunnerFeatureValue("obscuredInset.top"); }
double horizontalSystemMinimumLayoutMargin() const { return doubleTestRunnerFeatureValue("horizontalSystemMinimumLayoutMargin"); }
Expand Down
24 changes: 24 additions & 0 deletions Tools/WebKitTestRunner/ios/TestControllerIOS.mm
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,15 @@ static void overridePresentMenuOrPopoverOrViewController()
{
}

#if HAVE(UIKIT_RESIZABLE_WINDOWS)

static BOOL overrideEnhancedWindowingEnabled()
{
return YES;
}

#endif

namespace WTR {

static bool isDoneWaitingForKeyboardToStartDismissing = true;
Expand Down Expand Up @@ -255,6 +264,21 @@ static _WKFocusStartsInputSessionPolicy focusStartsInputSessionPolicy(const Test
{
cocoaResetStateToConsistentValues(options);

#if HAVE(UIKIT_RESIZABLE_WINDOWS)
bool enhancedWindowingStateChanged = false;
if (options.enhancedWindowingEnabled() && !m_enhancedWindowingEnabledSwizzler) {
m_enhancedWindowingEnabledSwizzler = WTF::makeUnique<InstanceMethodSwizzler>(UIWindowScene.class, @selector(_enhancedWindowingEnabled), reinterpret_cast<IMP>(overrideEnhancedWindowingEnabled));
enhancedWindowingStateChanged = true;
} else if (!options.enhancedWindowingEnabled() && m_enhancedWindowingEnabledSwizzler) {
m_enhancedWindowingEnabledSwizzler = nullptr;
enhancedWindowingStateChanged = true;
}
if (enhancedWindowingStateChanged) {
if (auto webView = mainWebView())
[[NSNotificationCenter defaultCenter] postNotificationName:_UIWindowSceneEnhancedWindowingModeChanged object:webView->platformView().window.windowScene userInfo:nil];
}
#endif // HAVE(UIKIT_RESIZABLE_WINDOWS)

[UIKeyboardImpl.activeInstance setCorrectionLearningAllowed:NO];
[pasteboardConsistencyEnforcer() clearPasteboard];
[[UIApplication sharedApplication] _cancelAllTouches];
Expand Down

0 comments on commit 85d3ed8

Please sign in to comment.