Commit d8fa6069 authored by Vasilii Sukhanov's avatar Vasilii Sukhanov Committed by Commit Bot

Revert "Let both minimum font-size settings apply to CSS px units."

This reverts commit bbbb1fad.

Reason for revert: broke WebTests on WebKit Mac10.13 (retina)
https://ci.chromium.org/p/chromium/builders/ci/WebKit%20Mac10.13%20%28retina%29/28885

Original change's description:
> Let both minimum font-size settings apply to CSS px units.
> 
> Applying the minimum font-size to the device pixels would not give the
> expected result, either for using zoom for DSF, or applying page zoom.
> 
> I added the documentation for the settings based on what I found on
> stackoverflow:
> 
> https://stackoverflow.com/questions/52943595/cannot-understand-android-webview-documentation
> 
> The behavior changes for both the non-"logical" setting in that we will
> still apply a minimum font-size even if the zoomed font-size would have
> become larger than the minimum font-size in device-pixels. That is, if
> we have a minimum font-size of 20px and two elements, one with a 10px
> font-size and the other with a 20px font-size, they will both be
> rendered at 40 device pixel size at 200% zoom.
> 
> This changes the behavior for the logical setting when we have a page
> zoom factor which is lower than 100%, where we also now just adjust the
> CSS px font-size before zooming is applied.
> 
> The logical setting is only exposed via the Android WebView API or via
> a hand-edited config file. The minimum font-size setting the
> chrome://settings is the non-"logical" one.
> 
> If the settings object is null, return the font-size without minimum
> sizes applied instead of the not-so-useful 1px return we had.
> 
> Bug: 1082127
> Change-Id: Idc03bf37fb7d39206ad4d4dc50eef01b56a69ddb
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2199134
> Reviewed-by: Philip Rogers <pdr@chromium.org>
> Reviewed-by: Dominik Röttsches <drott@chromium.org>
> Commit-Queue: Rune Lillesveen <futhark@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#770661}

TBR=pdr@chromium.org,chrishtr@chromium.org,drott@chromium.org,pdr@google.com,futhark@chromium.org

Change-Id: I70d34b6df1b42075781d4c29b753b7ae8ebe767c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1082127
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2210359Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#770977}
parent 57fc6ffd
...@@ -631,7 +631,6 @@ blink_core_tests("unit_tests") { ...@@ -631,7 +631,6 @@ blink_core_tests("unit_tests") {
"cssom/style_property_map_test.cc", "cssom/style_property_map_test.cc",
"drag_update_test.cc", "drag_update_test.cc",
"font_face_cache_test.cc", "font_face_cache_test.cc",
"font_size_functions_test.cc",
"font_update_invalidation_test.cc", "font_update_invalidation_test.cc",
"invalidation/invalidation_set_test.cc", "invalidation/invalidation_set_test.cc",
"invalidation/pending_invalidations_test.cc", "invalidation/pending_invalidations_test.cc",
......
...@@ -49,37 +49,44 @@ float FontSizeFunctions::GetComputedSizeFromSpecifiedSize( ...@@ -49,37 +49,44 @@ float FontSizeFunctions::GetComputedSizeFromSpecifiedSize(
if (fabsf(specified_size) < std::numeric_limits<float>::epsilon()) if (fabsf(specified_size) < std::numeric_limits<float>::epsilon())
return 0.0f; return 0.0f;
// We support two types of minimum font size. The first is a hard override
// that applies to all fonts. This is "minSize." The second type of minimum
// font size is a "smart minimum" that is applied only when the Web page can't
// know what size it really asked for, e.g., when it uses logical sizes like
// "small" or expresses the font-size as a percentage of the user's default
// font setting.
// With the smart minimum, we never want to get smaller than the minimum font
// size to keep fonts readable. However we always allow the page to set an
// explicit pixel size that is smaller, since sites will mis-render otherwise
// (e.g., http://www.gamespot.com with a 9px minimum).
Settings* settings = document->GetSettings(); Settings* settings = document->GetSettings();
if (apply_minimum_font_size && settings) { if (!settings)
// We support two types of minimum font size. The first is a hard override return 1.0f;
// that applies to all fonts. This is "min_size." The second type of minimum
// font size is a "smart minimum" that is applied only when the Web page
// can't know what size it really asked for, e.g., when it uses logical
// sizes like "small" or expresses the font-size as a percentage of the
// user's default font setting.
// With the smart minimum, we never want to get smaller than the minimum
// font size to keep fonts readable. However we always allow the page to set
// an explicit pixel size that is smaller, since sites will mis-render
// otherwise (e.g., http://www.gamespot.com with a 9px minimum).
float zoomed_size = specified_size * zoom_factor;
if (apply_minimum_font_size) {
int min_size = settings->GetMinimumFontSize(); int min_size = settings->GetMinimumFontSize();
int min_logical_size = settings->GetMinimumLogicalFontSize(); int min_logical_size = settings->GetMinimumLogicalFontSize();
// Apply the hard minimum first. // Apply the hard minimum first. We only apply the hard minimum if after
if (specified_size < min_size) // zooming we're still too small.
specified_size = min_size; if (zoomed_size < min_size)
zoomed_size = min_size;
// Now apply the "smart minimum". The font size must either be relative to // Now apply the "smart minimum." This minimum is also only applied if we're
// still too small after zooming. The font size must either be relative to
// the user default or the original size must have been acceptable. In other // the user default or the original size must have been acceptable. In other
// words, we only apply the smart minimum whenever we're positive doing so // words, we only apply the smart minimum whenever we're positive doing so
// won't disrupt the layout. // won't disrupt the layout.
if (specified_size < min_logical_size && !is_absolute_size) if (zoomed_size < min_logical_size &&
specified_size = min_logical_size; (specified_size >= min_logical_size || !is_absolute_size))
zoomed_size = min_logical_size;
} }
// Also clamp to a reasonable maximum to prevent insane font sizes from // Also clamp to a reasonable maximum to prevent insane font sizes from
// causing crashes on various platforms (I'm looking at you, Windows.) // causing crashes on various platforms (I'm looking at you, Windows.)
return std::min(kMaximumAllowedFontSize, specified_size * zoom_factor); return std::min(kMaximumAllowedFontSize, zoomed_size);
} }
const int kFontSizeTableMax = 16; const int kFontSizeTableMax = 16;
......
...@@ -23,7 +23,6 @@ ...@@ -23,7 +23,6 @@
#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_CSS_FONT_SIZE_FUNCTIONS_H_ #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_CSS_FONT_SIZE_FUNCTIONS_H_
#define THIRD_PARTY_BLINK_RENDERER_CORE_CSS_FONT_SIZE_FUNCTIONS_H_ #define THIRD_PARTY_BLINK_RENDERER_CORE_CSS_FONT_SIZE_FUNCTIONS_H_
#include "third_party/blink/renderer/core/core_export.h"
#include "third_party/blink/renderer/core/css_value_keywords.h" #include "third_party/blink/renderer/core/css_value_keywords.h"
#include "third_party/blink/renderer/platform/wtf/allocator/allocator.h" #include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
...@@ -36,7 +35,7 @@ enum ApplyMinimumFontSize { ...@@ -36,7 +35,7 @@ enum ApplyMinimumFontSize {
kApplyMinimumForFontSize kApplyMinimumForFontSize
}; };
class CORE_EXPORT FontSizeFunctions { class FontSizeFunctions {
STATIC_ONLY(FontSizeFunctions); STATIC_ONLY(FontSizeFunctions);
public: public:
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "third_party/blink/renderer/core/css/font_size_functions.h"
#include "third_party/blink/renderer/core/dom/document.h"
#include "third_party/blink/renderer/core/frame/settings.h"
#include "third_party/blink/renderer/core/testing/page_test_base.h"
namespace blink {
using FontSizeFunctionsTest = PageTestBase;
TEST_F(FontSizeFunctionsTest, GetComputedSizeFromSpecifiedSize_NoMinFontSize) {
constexpr float zoom_factor = 2;
constexpr int min_font_size = 100;
constexpr bool is_absolute = true;
constexpr bool is_logical = false;
GetDocument().GetSettings()->SetMinimumFontSize(min_font_size);
GetDocument().GetSettings()->SetMinimumLogicalFontSize(min_font_size);
for (const int& font_size : {1, 10, 40, 120}) {
EXPECT_EQ(font_size * zoom_factor,
FontSizeFunctions::GetComputedSizeFromSpecifiedSize(
&GetDocument(), zoom_factor, is_absolute, font_size,
kDoNotApplyMinimumForFontSize));
EXPECT_EQ(font_size * zoom_factor,
FontSizeFunctions::GetComputedSizeFromSpecifiedSize(
&GetDocument(), zoom_factor, is_logical, font_size,
kDoNotApplyMinimumForFontSize));
}
}
TEST_F(FontSizeFunctionsTest, GetComputedSizeFromSpecifiedSize_MinFontSize) {
constexpr float zoom_factor = 2;
constexpr int min_font_size = 100;
constexpr bool is_absolute = true;
constexpr bool is_logical = false;
GetDocument().GetSettings()->SetMinimumFontSize(min_font_size);
GetDocument().GetSettings()->SetMinimumLogicalFontSize(0);
int test_cases[][2] = {
{1, min_font_size}, {10, min_font_size}, {40, min_font_size}, {120, 120}};
for (const auto* font_sizes : test_cases) {
EXPECT_EQ(font_sizes[1] * zoom_factor,
FontSizeFunctions::GetComputedSizeFromSpecifiedSize(
&GetDocument(), zoom_factor, is_absolute, font_sizes[0]));
EXPECT_EQ(font_sizes[1] * zoom_factor,
FontSizeFunctions::GetComputedSizeFromSpecifiedSize(
&GetDocument(), zoom_factor, is_logical, font_sizes[0]));
}
}
TEST_F(FontSizeFunctionsTest,
GetComputedSizeFromSpecifiedSize_MinLogicalFontSize) {
constexpr float zoom_factor = 2;
constexpr int min_font_size = 100;
constexpr bool is_absolute = true;
constexpr bool is_logical = false;
GetDocument().GetSettings()->SetMinimumFontSize(0);
GetDocument().GetSettings()->SetMinimumLogicalFontSize(min_font_size);
int test_cases[][2] = {
{1, min_font_size}, {10, min_font_size}, {40, min_font_size}, {120, 120}};
for (const auto* font_sizes : test_cases) {
EXPECT_EQ(font_sizes[0] * zoom_factor,
FontSizeFunctions::GetComputedSizeFromSpecifiedSize(
&GetDocument(), zoom_factor, is_absolute, font_sizes[0]));
EXPECT_EQ(font_sizes[1] * zoom_factor,
FontSizeFunctions::GetComputedSizeFromSpecifiedSize(
&GetDocument(), zoom_factor, is_logical, font_sizes[0]));
}
}
} // namespace blink
...@@ -65,17 +65,12 @@ ...@@ -65,17 +65,12 @@
type: "int", type: "int",
}, },
// Sets the minimum font-size in CSS px. That means the minimum font-size is
// applied before applying the device scale factor or page zoom.
{ {
name: "minimumFontSize", name: "minimumFontSize",
initial: 0, initial: 0,
invalidate: "Style", invalidate: "Style",
type: "int", type: "int",
}, },
// Sets the minimum font-size in CSS px, but only applies to relative font
// units like 'small', em, and percentage sizes.
{ {
name: "minimumLogicalFontSize", name: "minimumLogicalFontSize",
initial: 0, initial: 0,
......
...@@ -3,6 +3,6 @@ ...@@ -3,6 +3,6 @@
<title>Maintain correct font-size in ex and ch with zoom</title> <title>Maintain correct font-size in ex and ch with zoom</title>
<body style="margin: 0; padding: 0;"> <body style="margin: 0; padding: 0;">
<div style="font-size: 6ex">size should be 6ex</div> <div style="font-size: 3ex">size should be 3ex</div>
<div style="font-size: 6ch">size should be 6ch</div> <div style="font-size: 3ch">size should be 3ch</div>
</body> </body>
...@@ -7,8 +7,8 @@ ...@@ -7,8 +7,8 @@
<title>Maintain correct font-size in ex and ch with zoom</title> <title>Maintain correct font-size in ex and ch with zoom</title>
<body style="margin: 0; padding: 0;"> <body style="margin: 0; padding: 0;">
<div style="font-size: 2ex">size should be 6ex</div> <div style="font-size: 1ex">size should be 3ex</div>
<div style="font-size: 2ch">size should be 6ch</div> <div style="font-size: 1ch">size should be 3ch</div>
</body> </body>
<script> <script>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment