WebKit Bugzilla
Attachment 356613 Details for
Bug 192377
: We should ignore minimumEffectiveDeviceWidth if the page specifies device-width in viewport meta-tag.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Address review comments.
ignore_minimum_effective_device_width.patch (text/plain), 18.99 KB, created by
Yongjun Zhang
on 2018-12-05 09:59:08 PST
(
hide
)
Description:
Address review comments.
Filename:
MIME Type:
Creator:
Yongjun Zhang
Created:
2018-12-05 09:59:08 PST
Size:
18.99 KB
patch
obsolete
>commit 70d81f7d6db8c2b39108f2d3e1e1c3f90edad1f2 >Author: Yongjun Zhang <yongjun_zhang@apple.com> >Date: Tue Dec 4 14:03:14 2018 -0800 > > We should ignore minimumEffectiveDeviceWidth if the page specifies device-width in viewport meta-tag. > https://bugs.webkit.org/show_bug.cgi?id=192377 > <rdar://problem/46364206> > >diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog >index 191953785cd..99b504313b7 100644 >--- a/LayoutTests/ChangeLog >+++ b/LayoutTests/ChangeLog >@@ -1,3 +1,16 @@ >+2018-12-04 Yongjun Zhang <yongjun_zhang@apple.com> >+ >+ We should ignore minimumEffectiveDeviceWidth if the page specifies device-width in viewport meta-tag. >+ https://bugs.webkit.org/show_bug.cgi?id=192377 >+ <rdar://problem/46364206> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * fast/viewport/ios/ignore-minimum-device-width-for-page-with-viewport-device-width-expected.txt: Added. >+ * fast/viewport/ios/ignore-minimum-device-width-for-page-with-viewport-device-width.html: Added. >+ * fast/viewport/ios/use-minimum-device-width-for-page-without-viewport-meta-expected.txt: Added. >+ * fast/viewport/ios/use-minimum-device-width-for-page-without-viewport-meta.html: Added. >+ > 2018-12-04 Ryan Haddad <ryanhaddad@apple.com> > > Unreviewed, rolling out r238838. >diff --git a/LayoutTests/fast/viewport/ios/ignore-minimum-device-width-for-page-with-viewport-device-width-expected.txt b/LayoutTests/fast/viewport/ios/ignore-minimum-device-width-for-page-with-viewport-device-width-expected.txt >new file mode 100644 >index 00000000000..823735f465d >--- /dev/null >+++ b/LayoutTests/fast/viewport/ios/ignore-minimum-device-width-for-page-with-viewport-device-width-expected.txt >@@ -0,0 +1,41 @@ >+setMinimumEffectiveWidth(640.00) >+window size: [320, 548] >+square size: [32, 55] >+zoom scale: 1.00 >+ >+setMinimumEffectiveWidth(768.00) >+window size: [320, 548] >+square size: [32, 55] >+zoom scale: 1.00 >+ >+setMinimumEffectiveWidth(834.00) >+window size: [320, 548] >+square size: [32, 55] >+zoom scale: 1.00 >+ >+setMinimumEffectiveWidth(980.00) >+window size: [320, 548] >+square size: [32, 55] >+zoom scale: 1.00 >+ >+setMinimumEffectiveWidth(1024.00) >+window size: [320, 548] >+square size: [32, 55] >+zoom scale: 1.00 >+ >+setMinimumEffectiveWidth(1112.00) >+window size: [320, 548] >+square size: [32, 55] >+zoom scale: 1.00 >+ >+setMinimumEffectiveWidth(1280.00) >+window size: [320, 548] >+square size: [32, 55] >+zoom scale: 1.00 >+ >+setMinimumEffectiveWidth(1336.00) >+window size: [320, 548] >+square size: [32, 55] >+zoom scale: 1.00 >+ >+ >diff --git a/LayoutTests/fast/viewport/ios/ignore-minimum-device-width-for-page-with-viewport-device-width.html b/LayoutTests/fast/viewport/ios/ignore-minimum-device-width-for-page-with-viewport-device-width.html >new file mode 100644 >index 00000000000..8022d985510 >--- /dev/null >+++ b/LayoutTests/fast/viewport/ios/ignore-minimum-device-width-for-page-with-viewport-device-width.html >@@ -0,0 +1,58 @@ >+<!DOCTYPE html> <!-- webkit-test-runner [ shouldIgnoreMetaViewport=true ] --> >+<html> >+<head> >+ <meta name="viewport" content="width=device-width"> >+ <style> >+ #square { >+ position: absolute; >+ width: 10vw; >+ height: 10vh; >+ border: 2px solid black; >+ } >+ >+ #output { >+ width: 100%; >+ height: 100%; >+ overflow: scroll; >+ } >+ >+ body { >+ margin: 0; >+ width: 100%; >+ height: 100%; >+ } >+ </style> >+ <script src="../../../resources/ui-helper.js"></script> >+ <script> >+ if (window.testRunner) { >+ testRunner.dumpAsText(); >+ testRunner.waitUntilDone(); >+ } >+ >+ async function runTest() { >+ const appendOutput = message => { >+ output.appendChild(document.createTextNode(message)); >+ output.appendChild(document.createElement("br")); >+ }; >+ >+ for (let targetMinimumEffectiveWidth of [640, 768, 834, 980, 1024, 1112, 1280, 1336]) { >+ appendOutput(`setMinimumEffectiveWidth(${targetMinimumEffectiveWidth.toFixed(2)})`); >+ >+ await UIHelper.setMinimumEffectiveWidth(targetMinimumEffectiveWidth); >+ await Promise.all([UIHelper.ensureVisibleContentRectUpdate(), UIHelper.ensurePresentationUpdate()]); >+ appendOutput(`window size: [${innerWidth}, ${innerHeight}]`); >+ appendOutput(`square size: [${square.clientWidth}, ${square.clientHeight}]`); >+ appendOutput(`zoom scale: ${(await UIHelper.zoomScale()).toFixed(2)}`); >+ appendOutput(""); >+ } >+ >+ testRunner.notifyDone(); >+ } >+ </script> >+</head> >+ >+<body onload="runTest()"> >+ <div id="square"></div> >+ <pre id="output"></pre> >+</body> >+</html> >diff --git a/LayoutTests/fast/viewport/ios/use-minimum-device-width-for-page-without-viewport-meta-expected.txt b/LayoutTests/fast/viewport/ios/use-minimum-device-width-for-page-without-viewport-meta-expected.txt >new file mode 100644 >index 00000000000..06d910e1050 >--- /dev/null >+++ b/LayoutTests/fast/viewport/ios/use-minimum-device-width-for-page-without-viewport-meta-expected.txt >@@ -0,0 +1,41 @@ >+setMinimumEffectiveWidth(640.00) >+window size: [640, 1096] >+square size: [64, 110] >+zoom scale: 0.50 >+ >+setMinimumEffectiveWidth(768.00) >+window size: [768, 1315] >+square size: [77, 132] >+zoom scale: 0.42 >+ >+setMinimumEffectiveWidth(834.00) >+window size: [834, 1428] >+square size: [83, 143] >+zoom scale: 0.38 >+ >+setMinimumEffectiveWidth(980.00) >+window size: [980, 1678] >+square size: [98, 168] >+zoom scale: 0.33 >+ >+setMinimumEffectiveWidth(1024.00) >+window size: [1024, 1754] >+square size: [102, 175] >+zoom scale: 0.31 >+ >+setMinimumEffectiveWidth(1112.00) >+window size: [1112, 1904] >+square size: [111, 190] >+zoom scale: 0.29 >+ >+setMinimumEffectiveWidth(1280.00) >+window size: [1280, 2192] >+square size: [128, 219] >+zoom scale: 0.25 >+ >+setMinimumEffectiveWidth(1336.00) >+window size: [1336, 2288] >+square size: [134, 229] >+zoom scale: 0.24 >+ >+ >diff --git a/LayoutTests/fast/viewport/ios/use-minimum-device-width-for-page-without-viewport-meta.html b/LayoutTests/fast/viewport/ios/use-minimum-device-width-for-page-without-viewport-meta.html >new file mode 100644 >index 00000000000..f2043d4d3ce >--- /dev/null >+++ b/LayoutTests/fast/viewport/ios/use-minimum-device-width-for-page-without-viewport-meta.html >@@ -0,0 +1,57 @@ >+<!DOCTYPE html> <!-- webkit-test-runner [ shouldIgnoreMetaViewport=true ] --> >+<html> >+<head> >+ <style> >+ #square { >+ position: absolute; >+ width: 10vw; >+ height: 10vh; >+ border: 2px solid black; >+ } >+ >+ #output { >+ width: 100%; >+ height: 100%; >+ overflow: scroll; >+ } >+ >+ body { >+ margin: 0; >+ width: 100%; >+ height: 100%; >+ } >+ </style> >+ <script src="../../../resources/ui-helper.js"></script> >+ <script> >+ if (window.testRunner) { >+ testRunner.dumpAsText(); >+ testRunner.waitUntilDone(); >+ } >+ >+ async function runTest() { >+ const appendOutput = message => { >+ output.appendChild(document.createTextNode(message)); >+ output.appendChild(document.createElement("br")); >+ }; >+ >+ for (let targetMinimumEffectiveWidth of [640, 768, 834, 980, 1024, 1112, 1280, 1336]) { >+ appendOutput(`setMinimumEffectiveWidth(${targetMinimumEffectiveWidth.toFixed(2)})`); >+ >+ await UIHelper.setMinimumEffectiveWidth(targetMinimumEffectiveWidth); >+ await Promise.all([UIHelper.ensureVisibleContentRectUpdate(), UIHelper.ensurePresentationUpdate()]); >+ appendOutput(`window size: [${innerWidth}, ${innerHeight}]`); >+ appendOutput(`square size: [${square.clientWidth}, ${square.clientHeight}]`); >+ appendOutput(`zoom scale: ${(await UIHelper.zoomScale()).toFixed(2)}`); >+ appendOutput(""); >+ } >+ >+ testRunner.notifyDone(); >+ } >+ </script> >+</head> >+ >+<body onload="runTest()"> >+ <div id="square"></div> >+ <pre id="output"></pre> >+</body> >+</html> >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index e6ee9ed4791..3865b3d579f 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,44 @@ >+2018-12-04 Yongjun Zhang <yongjun_zhang@apple.com> >+ >+ We should ignore minimumEffectiveDeviceWidth if the page specifies device-width in viewport meta-tag. >+ https://bugs.webkit.org/show_bug.cgi?id=192377 >+ <rdar://problem/46364206> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ If the page specifies width=device-width or initial-scale=1 in the viewport meta tag, we should use the >+ native device width and ignore the minimum effective device width in ViewportConfiguration. The patch >+ also introduces standardWebpageParameters() which uses the device width as default and also allows the page >+ to shrink-to-fit. If a page doesn't have viewport meta tag, or if the width argument isn't device-width >+ and the initial scale isn't 1, we will use standardWebpageParameters() as the default configuration. >+ >+ Tests: fast/viewport/ios/ignore-minimum-device-width-for-page-with-viewport-device-width.html >+ fast/viewport/ios/use-minimum-device-width-for-page-without-viewport-meta.html >+ >+ * page/ViewportConfiguration.cpp: >+ (WebCore::ViewportConfiguration::updateDefaultConfiguration): pick the default configuration based on >+ the page's viewport arguments. Also, we will always fall back to standardWebpageParameters() if we >+ can ignore scaling constraints. >+ (WebCore::ViewportConfiguration::setViewportArguments): When page sends us new ViewportArguments, pick >+ up the correponsding default configuration before updating the configuration. >+ (WebCore::ViewportConfiguration::setCanIgnoreScalingConstraints): When m_canIgnoreScalingConstraints is >+ changed, try to pick up the correponsding default configuration. >+ (WebCore::ViewportConfiguration::standardWebpageParameters): Add a new default set of viewport Parameters >+ this is very close to nativeWebpageParameters() excpet that it allows shrink to fit and its minimum scale >+ is 0.25. We will use this Parameters for pages that doesn't have viewport meta tag; or the width is >+ not device-width and initial scale is not 1. >+ (WebCore::ViewportConfiguration::updateConfiguration): If the page's viewport argument doesn't override >+ the default width, use the m_minimumLayoutSize.width(). >+ * page/ViewportConfiguration.h: >+ (WebCore::ViewportConfiguration::shouldIgnoreMinimumEffectiveDeviceWidth const): A helper method to tell >+ if we should avoid using minimum effective device width. >+ (WebCore::ViewportConfiguration::canOverrideConfigurationParameters const): If we are using a default >+ configuration that is neither nativeWebpageParameters() nor standardWebpageParameters(), don't override >+ it. >+ (WebCore::ViewportConfiguration::minimumEffectiveDeviceWidth const): Add a helper method to return minimum >+ effective device width based on shouldIgnoreMinimumEffectiveDeviceWidth(). >+ (WebCore::ViewportConfiguration::effectiveLayoutSizeScaleFactor const): Use minimumEffectiveDeviceWidth(). >+ > 2018-12-04 Chris Dumez <cdumez@apple.com> > > Regression(r238817) PSON Page Cache API tests are failing >diff --git a/Source/WebCore/page/ViewportConfiguration.cpp b/Source/WebCore/page/ViewportConfiguration.cpp >index 5cc814087b6..373cdade17c 100644 >--- a/Source/WebCore/page/ViewportConfiguration.cpp >+++ b/Source/WebCore/page/ViewportConfiguration.cpp >@@ -143,6 +143,27 @@ bool ViewportConfiguration::setDisabledAdaptations(const OptionSet<DisabledAdapt > return true; > } > >+bool ViewportConfiguration::canOverrideConfigurationParameters() const >+{ >+ return m_defaultConfiguration == ViewportConfiguration::nativeWebpageParameters() || m_defaultConfiguration == ViewportConfiguration::standardWebpageParameters(); >+} >+ >+void ViewportConfiguration::updateDefaultConfiguration() >+{ >+ if (!canOverrideConfigurationParameters()) >+ return; >+ >+ if (m_canIgnoreScalingConstraints) { >+ m_defaultConfiguration = ViewportConfiguration::standardWebpageParameters(); >+ return; >+ } >+ >+ if (shouldIgnoreMinimumEffectiveDeviceWidth()) >+ m_defaultConfiguration = ViewportConfiguration::nativeWebpageParameters(); >+ else >+ m_defaultConfiguration = ViewportConfiguration::standardWebpageParameters(); >+} >+ > bool ViewportConfiguration::setViewportArguments(const ViewportArguments& viewportArguments) > { > if (m_viewportArguments == viewportArguments) >@@ -151,6 +172,7 @@ bool ViewportConfiguration::setViewportArguments(const ViewportArguments& viewpo > LOG_WITH_STREAM(Viewports, stream << "ViewportConfiguration::setViewportArguments " << viewportArguments); > m_viewportArguments = viewportArguments; > >+ updateDefaultConfiguration(); > updateMinimumLayoutSize(); > updateConfiguration(); > return true; >@@ -162,6 +184,7 @@ bool ViewportConfiguration::setCanIgnoreScalingConstraints(bool canIgnoreScaling > return false; > > m_canIgnoreScalingConstraints = canIgnoreScalingConstraints; >+ updateDefaultConfiguration(); > updateConfiguration(); > return true; > } >@@ -325,6 +348,21 @@ ViewportConfiguration::Parameters ViewportConfiguration::nativeWebpageParameters > return parameters; > } > >+ViewportConfiguration::Parameters ViewportConfiguration::standardWebpageParameters() >+{ >+ Parameters parameters; >+ parameters.width = ViewportArguments::ValueDeviceWidth; >+ parameters.widthIsSet = true; >+ parameters.allowsUserScaling = true; >+ parameters.allowsShrinkToFit = true; >+ parameters.minimumScale = 0.25; >+ parameters.maximumScale = 5; >+ parameters.initialScale = 1; >+ parameters.initialScaleIsSet = true; >+ parameters.initialScaleIgnoringLayoutScaleFactor = 1; >+ return parameters; >+} >+ > ViewportConfiguration::Parameters ViewportConfiguration::webpageParameters() > { > Parameters parameters; >@@ -448,6 +486,9 @@ void ViewportConfiguration::updateConfiguration() > else if (booleanViewportArgumentIsSet(m_viewportArguments.shrinkToFit)) > m_configuration.allowsShrinkToFit = m_viewportArguments.shrinkToFit != 0.; > >+ if (canOverrideConfigurationParameters() && !viewportArgumentsOverridesWidth) >+ m_configuration.width = m_minimumLayoutSize.width(); >+ > m_configuration.avoidsUnsafeArea = m_viewportArguments.viewportFit != ViewportFit::Cover; > m_configuration.initialScaleIgnoringLayoutScaleFactor = m_configuration.initialScale; > float effectiveLayoutScale = effectiveLayoutSizeScaleFactor(); >diff --git a/Source/WebCore/page/ViewportConfiguration.h b/Source/WebCore/page/ViewportConfiguration.h >index 4987be2c645..7380b7277d3 100644 >--- a/Source/WebCore/page/ViewportConfiguration.h >+++ b/Source/WebCore/page/ViewportConfiguration.h >@@ -103,6 +103,7 @@ public: > > // Matches a width=device-width, initial-scale=1 viewport. > WEBCORE_EXPORT static Parameters nativeWebpageParameters(); >+ static Parameters standardWebpageParameters(); > WEBCORE_EXPORT static Parameters webpageParameters(); > WEBCORE_EXPORT static Parameters textDocumentParameters(); > WEBCORE_EXPORT static Parameters imageDocumentParameters(); >@@ -126,6 +127,29 @@ private: > bool shouldIgnoreScalingConstraints() const; > bool shouldIgnoreVerticalScalingConstraints() const; > bool shouldIgnoreHorizontalScalingConstraints() const; >+ void updateDefaultConfiguration(); >+ bool canOverrideConfigurationParameters() const; >+ >+ constexpr bool shouldIgnoreMinimumEffectiveDeviceWidth() const >+ { >+ if (m_canIgnoreScalingConstraints) >+ return true; >+ >+ if (m_viewportArguments == ViewportArguments()) >+ return false; >+ >+ if (m_viewportArguments.width == ViewportArguments::ValueDeviceWidth || m_viewportArguments.zoom == 1.) >+ return true; >+ >+ return false; >+ } >+ >+ constexpr double minimumEffectiveDeviceWidth() const >+ { >+ if (shouldIgnoreMinimumEffectiveDeviceWidth()) >+ return 0; >+ return m_minimumEffectiveDeviceWidth; >+ } > > constexpr double forceAlwaysUserScalableMaximumScale() const > { >@@ -141,9 +165,9 @@ private: > > constexpr double effectiveLayoutSizeScaleFactor() const > { >- if (!m_viewLayoutSize.width() || !m_minimumEffectiveDeviceWidth) >+ if (!m_viewLayoutSize.width() || !minimumEffectiveDeviceWidth()) > return m_layoutSizeScaleFactor; >- return m_layoutSizeScaleFactor * m_viewLayoutSize.width() / std::max<double>(m_minimumEffectiveDeviceWidth, m_viewLayoutSize.width()); >+ return m_layoutSizeScaleFactor * m_viewLayoutSize.width() / std::max<double>(minimumEffectiveDeviceWidth(), m_viewLayoutSize.width()); > } > > void updateMinimumLayoutSize(); >diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog >index 55dff1c019a..cc92160c687 100644 >--- a/Source/WebKit/ChangeLog >+++ b/Source/WebKit/ChangeLog >@@ -1,3 +1,19 @@ >+2018-12-04 Yongjun Zhang <yongjun_zhang@apple.com> >+ >+ We should ignore minimumEffectiveDeviceWidth if the page specifies device-width in viewport meta-tag. >+ https://bugs.webkit.org/show_bug.cgi?id=192377 >+ <rdar://problem/46364206> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Since we are using page's viewport arguments to decide the default viewport parameters and >+ whether we can use mininum effective device width, we should always call setViewportArguments() >+ regardless of shouldIgnoreMetaViewport settings. >+ >+ * WebProcess/WebPage/WebPage.cpp: >+ (WebKit::WebPage::viewportPropertiesDidChange): Always call setViewportArguments(). >+ (WebKit::WebPage::didCommitLoad): Ditto. >+ > 2018-12-04 Chris Dumez <cdumez@apple.com> > > Regression(r238817) PSON Page Cache API tests are failing >diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp >index ac475152d74..6f2ca1f5093 100644 >--- a/Source/WebKit/WebProcess/WebPage/WebPage.cpp >+++ b/Source/WebKit/WebProcess/WebPage/WebPage.cpp >@@ -1926,7 +1926,7 @@ void WebPage::disabledAdaptationsDidChange(const OptionSet<DisabledAdaptations>& > void WebPage::viewportPropertiesDidChange(const ViewportArguments& viewportArguments) > { > #if PLATFORM(IOS_FAMILY) >- if (!m_page->settings().shouldIgnoreMetaViewport() && m_viewportConfiguration.setViewportArguments(viewportArguments)) >+ if (m_viewportConfiguration.setViewportArguments(viewportArguments)) > viewportConfigurationChanged(); > #endif > >@@ -5541,7 +5541,7 @@ void WebPage::didCommitLoad(WebFrame* frame) > if (m_viewportConfiguration.setContentsSize(coreFrame->view()->contentsSize())) > viewportChanged = true; > >- if (!m_page->settings().shouldIgnoreMetaViewport() && m_viewportConfiguration.setViewportArguments(coreFrame->document()->viewportArguments())) >+ if (m_viewportConfiguration.setViewportArguments(coreFrame->document()->viewportArguments())) > viewportChanged = true; > > if (viewportChanged)
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Flags:
thorton
:
review+
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 192377
:
356548
|
356613
|
356762