Skip to content

Commit

Permalink
Initial scale of a page is not respected when both initial scale and …
Browse files Browse the repository at this point in the history
…viewport width are set

https://bugs.webkit.org/show_bug.cgi?id=258663
rdar://111515914

Reviewed by Wenson Hsieh.

In 256476@main, we introduce a viewport sizing heuristic to clamp the
maximum initial scale of a page. This was necessitated by Wikipedia's
dynamic initial scale assignment (to maintain their viewport width
invariant of 1000) leading to Wiki articles being excessively zoomed in
on large screens (such as when an iPad is connected to an external
display).

However, the somewhat arbitrarily chosen initial scale cap of 1.2 leads
to a sub-optimal web development experience because we perform the
initial scale clamping on every page (when the `IgnoreMetaViewport` is
enabled), meaning it isn't clear why the initial scale is not being
respected, even when a developer may have had a valid use-case.

To mitigate this situation, we only apply this initial scale clamping as
a quirk for pages on the "wikipedia.org" domain. This is a relatively
safe decision given that Wikipedia is probably the only top site that
performs a dynamic initial scale assignment (that too with potentially
*large* scale values).

We also introduce an API test to assert this behavior is only limited to
Wikipedia.

* LayoutTests/fast/viewport/ios/adjust-fixed-width-and-initial-scale-to-avoid-excessive-zooming.html: Removed.

This layout test is no longer valid.

* Source/WebCore/page/Quirks.cpp:
(WebCore::isWikipediaDomain):
(WebCore::Quirks::shouldIgnoreViewportArgumentsToAvoidExcessiveZoom const):
(WebCore::Quirks::shouldLayOutAtMinimumWindowWidthWhenIgnoringScalingConstraints const):
* Source/WebCore/page/Quirks.h:
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::setCanIgnoreViewportArgumentsToAvoidExcessiveZoomIfNeeded):
(WebKit::WebPage::didCommitLoad):
(WebKit::WebPage::updateWebsitePolicies):
* Tools/TestWebKitAPI/Tests/ios/Viewport.mm:
(TestWebKitAPI::makeViewportMetaTag):
(TestWebKitAPI::TEST):

Canonical link: https://commits.webkit.org/265804@main
  • Loading branch information
aprotyas committed Jul 6, 2023
1 parent 69d5cb6 commit 524f1bc
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 38 deletions.

This file was deleted.

20 changes: 19 additions & 1 deletion Source/WebCore/page/Quirks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -811,6 +811,24 @@ bool Quirks::shouldIgnoreAriaForFastPathContentObservationCheck() const
return false;
}

static bool isWikipediaDomain(const URL& url)
{
static NeverDestroyed wikipediaDomain = RegistrableDomain { URL { "https://wikipedia.org"_s } };
return wikipediaDomain->matches(url);
}

// wikipedia.org https://webkit.org/b/247636
bool Quirks::shouldIgnoreViewportArgumentsToAvoidExcessiveZoom() const
{
if (!needsQuirks())
return false;

#if ENABLE(META_VIEWPORT)
return isWikipediaDomain(m_document->url());
#endif
return false;
}

// docs.google.com https://bugs.webkit.org/show_bug.cgi?id=199933
bool Quirks::shouldOpenAsAboutBlank(const String& stringToOpen) const
{
Expand Down Expand Up @@ -1020,7 +1038,7 @@ bool Quirks::shouldLayOutAtMinimumWindowWidthWhenIgnoringScalingConstraints() co

// FIXME: We should consider replacing this with a heuristic to determine whether
// or not the edges of the page mostly lack content after shrinking to fit.
return m_document->url().host().endsWithIgnoringASCIICase(".wikipedia.org"_s);
return isWikipediaDomain(m_document->url());
}

// shutterstock.com rdar://58843932
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/page/Quirks.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ class Quirks {
WEBCORE_EXPORT bool shouldAvoidScrollingWhenFocusedContentIsVisible() const;
WEBCORE_EXPORT bool shouldUseLegacySelectPopoverDismissalBehaviorInDataActivation() const;
WEBCORE_EXPORT bool shouldIgnoreAriaForFastPathContentObservationCheck() const;
WEBCORE_EXPORT bool shouldIgnoreViewportArgumentsToAvoidExcessiveZoom() const;
WEBCORE_EXPORT bool shouldLayOutAtMinimumWindowWidthWhenIgnoringScalingConstraints() const;
WEBCORE_EXPORT bool shouldIgnoreContentObservationForSyntheticClick(bool isFirstSyntheticClickOnPage) const;
WEBCORE_EXPORT static bool shouldAllowNavigationToCustomProtocolWithoutUserGesture(StringView protocol, const SecurityOriginData& requesterOrigin);
Expand Down
12 changes: 10 additions & 2 deletions Source/WebKit/WebProcess/WebPage/WebPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7113,6 +7113,14 @@ void WebPage::didReplaceMultipartContent(const WebFrame& frame)
#endif
}

#if ENABLE(META_VIEWPORT)
static void setCanIgnoreViewportArgumentsToAvoidExcessiveZoomIfNeeded(ViewportConfiguration& configuration, LocalFrame* frame, bool shouldIgnoreMetaViewport)
{
if (auto* document = frame ? frame->document() : nullptr; document && document->quirks().shouldIgnoreViewportArgumentsToAvoidExcessiveZoom())
configuration.setCanIgnoreViewportArgumentsToAvoidExcessiveZoom(shouldIgnoreMetaViewport);
}
#endif

void WebPage::didCommitLoad(WebFrame* frame)
{
#if PLATFORM(IOS_FAMILY)
Expand Down Expand Up @@ -7198,7 +7206,7 @@ void WebPage::didCommitLoad(WebFrame* frame)

bool viewportChanged = false;

m_viewportConfiguration.setCanIgnoreViewportArgumentsToAvoidExcessiveZoom(shouldIgnoreMetaViewport());
setCanIgnoreViewportArgumentsToAvoidExcessiveZoomIfNeeded(m_viewportConfiguration, coreFrame, shouldIgnoreMetaViewport());
m_viewportConfiguration.setPrefersHorizontalScrollingBelowDesktopViewportWidths(shouldEnableViewportBehaviorsForResizableWindows());

LOG_WITH_STREAM(VisibleRects, stream << "WebPage " << m_identifier.toUInt64() << " didCommitLoad setting content size to " << coreFrame->view()->contentsSize());
Expand Down Expand Up @@ -7434,7 +7442,7 @@ void WebPage::updateWebsitePolicies(WebsitePoliciesData&& websitePolicies)
#endif

#if ENABLE(META_VIEWPORT)
m_viewportConfiguration.setCanIgnoreViewportArgumentsToAvoidExcessiveZoom(shouldIgnoreMetaViewport());
setCanIgnoreViewportArgumentsToAvoidExcessiveZoomIfNeeded(m_viewportConfiguration, localMainFrame, shouldIgnoreMetaViewport());
#endif
}

Expand Down
49 changes: 48 additions & 1 deletion Tools/TestWebKitAPI/Tests/ios/Viewport.mm
Original file line number Diff line number Diff line change
Expand Up @@ -27,24 +27,32 @@

#if PLATFORM(IOS_FAMILY)

#import "HTTPServer.h"
#import "PlatformUtilities.h"
#import "Test.h"
#import "TestNavigationDelegate.h"
#import "TestWKWebView.h"
#import "UserInterfaceSwizzler.h"
#import <WebKit/WKPreferencesPrivate.h>
#import <WebKit/WKWebView.h>
#import <WebKit/WKWebViewPrivateForTesting.h>
#import <wtf/BlockPtr.h>
#import <wtf/RetainPtr.h>

namespace TestWebKitAPI {

static inline String makeViewportMetaTag(unsigned viewportWidth, float initialScale)
{
return makeString("<meta name='viewport' content='width=", viewportWidth, ", initial-scale=", initialScale, "'>");
}

TEST(Viewport, MinimumEffectiveDeviceWidthWithInitialScale)
{
IPadUserInterfaceSwizzler iPadUserInterface;

auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 690, 690)]);

[webView synchronouslyLoadHTMLString:@"<meta name='viewport' content='width=1000,initial-scale=0.69'>"];
[webView synchronouslyLoadHTMLString:makeViewportMetaTag(1000, 0.69)];
[webView waitForNextPresentationUpdate];

[webView _setMinimumEffectiveDeviceWidth:1024];
Expand All @@ -53,6 +61,45 @@
EXPECT_EQ(static_cast<NSNumber *>([webView objectByEvaluatingJavaScript:@"window.innerWidth"]).floatValue, 1024);
}

TEST(Viewport, RespectInitialScaleExceptOnWikipediaDomain)
{
IPadUserInterfaceSwizzler iPadUserInterface;

auto screenDimensions = CGRectMake(0, 0, 2732, 2048);
auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:screenDimensions]);
[[webView configuration] preferences]._shouldIgnoreMetaViewport = YES;
auto navigationDelegate = adoptNS([TestNavigationDelegate new]);
[webView setNavigationDelegate:navigationDelegate.get()];

auto runTest = [&webView, &navigationDelegate, screenWidth = screenDimensions.size.width] (StringView requestURL, Function<void(float, float)>&& scaleExpectation) mutable {
bool done = false;
constexpr auto viewportWidth = 1000;
auto expectedViewportScale = screenWidth / viewportWidth;

auto request = [NSURLRequest requestWithURL:[NSURL URLWithString:[NSString stringWithUTF8String:requestURL.utf8().data()]]];
auto response = makeViewportMetaTag(viewportWidth, expectedViewportScale);
[webView loadSimulatedRequest:request responseHTMLString:response];
[navigationDelegate waitForDidFinishNavigation];
[webView _doAfterNextPresentationUpdate:makeBlockPtr([&done, &webView, expectedViewportScale, scaleExpectation = WTFMove(scaleExpectation)]() mutable {
[webView evaluateJavaScript:@"visualViewport.scale" completionHandler:makeBlockPtr([&done, expectedViewportScale, scaleExpectation = WTFMove(scaleExpectation)] (NSNumber *value, NSError *error) mutable {
auto actualViewportScale = value.floatValue;
scaleExpectation(actualViewportScale, expectedViewportScale);
done = true;
}).get()];
}).get()];

TestWebKitAPI::Util::run(&done);
done = false;
};

runTest("https://en.wikipedia.org/path1"_s, [] (float actualViewportScale, float expectedViewportScale) {
EXPECT_LT(actualViewportScale, expectedViewportScale);
});
runTest("https://webkit.org/path2"_s, [] (float actualViewportScale, float expectedViewportScale) {
EXPECT_EQ(actualViewportScale, expectedViewportScale);
});
}

}

#endif

0 comments on commit 524f1bc

Please sign in to comment.