Skip to content

GitLab

T
tangled
Commit 1229b141 authored by pdr's avatar pdr Committed by Commit bot
Browse files
Options

(reland) Do not let text-size-adjust override accessibility font settings 

The text autosizer is used for both making desktop pages more legible
on mobile, and for applying the accessibility font scale factor. When
text-size-adjust support was added, pages were able to override the
accessibility setting in addition to the autosizing multiplier.

This patch ensures accessibility font scale settings are respected even
with text-size-adjust. A followup bug has been filed (crbug.com/645717)
for moving the accessibility font scale factor out of the autosizer.

This patch also re-enables support for text-size-adjust which was
disabled temporarily due to breaking accessibility settings.

This was reverted due to a forgotten "!" in checking whether the
viewport was specified by the author in TextAutosizer.cpp. A new test
has been added to catch this in the future. The unneeded meta viewport
declarations in all other tests have been removed because they had
no effect with the meta viewport setting disabled.

Original review:
https://codereview.chromium.org/2329733002

BUG=645269,646342

Review-Url: https://codereview.chromium.org/2340553003
Cr-Commit-Position: refs/heads/master@{#418488}
parent 2f83bcda
Hide whitespace changes
Inline Side-by-side
Showing with 187 additions and 33 deletions
third_party/WebKit/Source/core/layout/TextAutosizer.cpp
View file @ 1229b141
......@@ -109,8 +109,8 @@ void TextAutosizer::writeClusterDebugInfo(Cluster* cluster)
}
String pageInfo = "";
if (cluster->m_root->isLayoutView()) {
pageInfo = String::format("; pageinfo: bm %f * (lw %d / fw %d)",
m_pageInfo.m_baseMultiplier, m_pageInfo.m_layoutWidth, m_pageInfo.m_frameWidth);
pageInfo = String::format("; pageinfo: afsf %f * dsa %f * (lw %d / fw %d)",
m_pageInfo.m_accessibilityFontScaleFactor, m_pageInfo.m_deviceScaleAdjustment, m_pageInfo.m_layoutWidth, m_pageInfo.m_frameWidth);
}
float multiplier = cluster->m_flags & SUPPRESSING ? 1.0 : cluster->m_multiplier;
writeDebugInfo(const_cast<LayoutBlock*>(cluster->m_root),
......@@ -552,24 +552,27 @@ void TextAutosizer::updatePageInfo()
IntSize layoutSize = mainFrame->view()->layoutSize();
m_pageInfo.m_layoutWidth = horizontalWritingMode ? layoutSize.width() : layoutSize.height();
// Compute the base font scale multiplier based on device and accessibility settings.
m_pageInfo.m_baseMultiplier = m_document->settings()->accessibilityFontScaleFactor();
// TODO(pdr): Accessibility should be moved out of the text autosizer. See: crbug.com/645717.
m_pageInfo.m_accessibilityFontScaleFactor = m_document->settings()->accessibilityFontScaleFactor();
// If the page has a meta viewport or @viewport, don't apply the device scale adjustment.
const ViewportDescription& viewportDescription = mainFrame->document()->viewportDescription();
if (!viewportDescription.isSpecifiedByAuthor()) {
float deviceScaleAdjustment = m_document->settings()->deviceScaleAdjustment();
m_pageInfo.m_baseMultiplier *= deviceScaleAdjustment;
}
if (!mainFrame->document()->viewportDescription().isSpecifiedByAuthor())
m_pageInfo.m_deviceScaleAdjustment = m_document->settings()->deviceScaleAdjustment();
else
m_pageInfo.m_deviceScaleAdjustment = 1.0f;
// TODO(pdr): pageNeedsAutosizing should take into account whether text-size-adjust is used
// anywhere on the page because that also needs to trigger autosizing. See: crbug.com/646237.
m_pageInfo.m_pageNeedsAutosizing = !!m_pageInfo.m_frameWidth
&& (m_pageInfo.m_baseMultiplier * (static_cast<float>(m_pageInfo.m_layoutWidth) / m_pageInfo.m_frameWidth) > 1.0f);
&& (m_pageInfo.m_accessibilityFontScaleFactor * m_pageInfo.m_deviceScaleAdjustment * (static_cast<float>(m_pageInfo.m_layoutWidth) / m_pageInfo.m_frameWidth) > 1.0f);
}
if (m_pageInfo.m_pageNeedsAutosizing) {
// If page info has changed, multipliers may have changed. Force a layout to recompute them.
if (m_pageInfo.m_frameWidth != previousPageInfo.m_frameWidth
|| m_pageInfo.m_layoutWidth != previousPageInfo.m_layoutWidth
|| m_pageInfo.m_baseMultiplier != previousPageInfo.m_baseMultiplier
|| m_pageInfo.m_accessibilityFontScaleFactor != previousPageInfo.m_accessibilityFontScaleFactor
|| m_pageInfo.m_deviceScaleAdjustment != previousPageInfo.m_deviceScaleAdjustment
|| m_pageInfo.m_settingEnabled != previousPageInfo.m_settingEnabled)
setAllTextNeedsLayout();
} else if (previousPageInfo.m_hasAutosized) {
......@@ -901,9 +904,10 @@ float TextAutosizer::multiplierFromBlock(const LayoutBlock* block)
// Block width, in CSS pixels.
float blockWidth = widthFromBlock(block);
float multiplier = m_pageInfo.m_frameWidth ? std::min(blockWidth, static_cast<float>(m_pageInfo.m_layoutWidth)) / m_pageInfo.m_frameWidth : 1.0f;
return std::max(m_pageInfo.m_baseMultiplier * multiplier, 1.0f);
float layoutWidth = std::min(blockWidth, static_cast<float>(m_pageInfo.m_layoutWidth));
float multiplier = m_pageInfo.m_frameWidth ? layoutWidth / m_pageInfo.m_frameWidth : 1.0f;
multiplier *= m_pageInfo.m_accessibilityFontScaleFactor * m_pageInfo.m_deviceScaleAdjustment;
return std::max(multiplier, 1.0f);
}
const LayoutBlock* TextAutosizer::deepestBlockContainingAllText(Cluster* cluster)
......@@ -988,10 +992,10 @@ void TextAutosizer::applyMultiplier(LayoutObject* layoutObject, float multiplier
{
ASSERT(layoutObject);
ComputedStyle& currentStyle = layoutObject->mutableStyleRef();
// TODO(pdr): text-size-adjust is temporarily not honored due to
// breaking accessibility settings. See: https://645269.
if (false && !currentStyle.getTextSizeAdjust().isAuto()) {
multiplier = currentStyle.getTextSizeAdjust().multiplier();
if (!currentStyle.getTextSizeAdjust().isAuto()) {
// The accessibility font scale factor is applied by the autosizer so we need to apply that
// scale factor on top of the text-size-adjust multiplier.
multiplier = currentStyle.getTextSizeAdjust().multiplier() * m_pageInfo.m_accessibilityFontScaleFactor;
} else if (multiplier < 1) {
// Unlike text-size-adjust, the text autosizer should only inflate fonts.
multiplier = 1;
......
third_party/WebKit/Source/core/layout/TextAutosizer.h
View file @ 1229b141
......@@ -237,7 +237,8 @@ private:
PageInfo()
: m_frameWidth(0)
, m_layoutWidth(0)
, m_baseMultiplier(0)
, m_accessibilityFontScaleFactor(1)
, m_deviceScaleAdjustment(1)
, m_pageNeedsAutosizing(false)
, m_hasAutosized(false)
, m_settingEnabled(false)
......@@ -246,7 +247,8 @@ private:
int m_frameWidth; // LocalFrame width in density-independent pixels (DIPs).
int m_layoutWidth; // Layout width in CSS pixels.
float m_baseMultiplier; // Includes accessibility font scale factor and device scale adjustment.
float m_accessibilityFontScaleFactor;
float m_deviceScaleAdjustment;
bool m_pageNeedsAutosizing;
bool m_hasAutosized;
bool m_settingEnabled;
......
third_party/WebKit/Source/core/layout/TextAutosizerTest.cpp
View file @ 1229b141
......@@ -20,7 +20,6 @@ private:
TEST_F(TextAutosizerTest, SimpleParagraph)
{
setBodyInnerHTML(
"<meta name='viewport' content='width=800'>"
"<style>"
" html { font-size: 16px; }"
" body { width: 800px; margin: 0; overflow-y: hidden; }"
......@@ -39,10 +38,9 @@ TEST_F(TextAutosizerTest, SimpleParagraph)
EXPECT_FLOAT_EQ(40.f, autosized->layoutObject()->style()->computedFontSize());
}
TEST_F(TextAutosizerTest, DISABLED_TextSizeAdjustDisablesAutosizing)
TEST_F(TextAutosizerTest, TextSizeAdjustDisablesAutosizing)
{
setBodyInnerHTML(
"<meta name='viewport' content='width=800'>"
"<style>"
" html { font-size: 16px; }"
" body { width: 800px; margin: 0; overflow-y: hidden; }"
......@@ -82,10 +80,9 @@ TEST_F(TextAutosizerTest, DISABLED_TextSizeAdjustDisablesAutosizing)
EXPECT_FLOAT_EQ(16.f, textSizeAdjust100->style()->computedFontSize());
}
TEST_F(TextAutosizerTest, DISABLED_ParagraphWithChangingTextSizeAdjustment)
TEST_F(TextAutosizerTest, ParagraphWithChangingTextSizeAdjustment)
{
setBodyInnerHTML(
"<meta name='viewport' content='width=800'>"
"<style>"
" html { font-size: 16px; }"
" body { width: 800px; margin: 0; overflow-y: hidden; }"
......@@ -126,10 +123,9 @@ TEST_F(TextAutosizerTest, DISABLED_ParagraphWithChangingTextSizeAdjustment)
EXPECT_FLOAT_EQ(40.f, autosizedDiv->layoutObject()->style()->computedFontSize());
}
TEST_F(TextAutosizerTest, DISABLED_ZeroTextSizeAdjustment)
TEST_F(TextAutosizerTest, ZeroTextSizeAdjustment)
{
setBodyInnerHTML(
"<meta name='viewport' content='width=800'>"
"<style>"
" html { font-size: 16px; }"
" body { width: 800px; margin: 0; overflow-y: hidden; }"
......@@ -150,7 +146,6 @@ TEST_F(TextAutosizerTest, DISABLED_ZeroTextSizeAdjustment)
TEST_F(TextAutosizerTest, NegativeTextSizeAdjustment)
{
setBodyInnerHTML(
"<meta name='viewport' content='width=800'>"
"<style>"
" html { font-size: 16px; }"
" body { width: 800px; margin: 0; overflow-y: hidden; }"
......@@ -172,7 +167,6 @@ TEST_F(TextAutosizerTest, NegativeTextSizeAdjustment)
TEST_F(TextAutosizerTest, TextSizeAdjustmentPixelUnits)
{
setBodyInnerHTML(
"<meta name='viewport' content='width=800'>"
"<style>"
" html { font-size: 16px; }"
" body { width: 800px; margin: 0; overflow-y: hidden; }"
......@@ -191,10 +185,9 @@ TEST_F(TextAutosizerTest, TextSizeAdjustmentPixelUnits)
EXPECT_FLOAT_EQ(40.f, textSizeAdjustPixels->style()->computedFontSize());
}
TEST_F(TextAutosizerTest, DISABLED_NestedTextSizeAdjust)
TEST_F(TextAutosizerTest, NestedTextSizeAdjust)
{
setBodyInnerHTML(
"<meta name='viewport' content='width=800'>"
"<style>"
" html { font-size: 16px; }"
" body { width: 800px; margin: 0; overflow-y: hidden; }"
......@@ -225,10 +218,9 @@ TEST_F(TextAutosizerTest, DISABLED_NestedTextSizeAdjust)
EXPECT_FLOAT_EQ(8.48f, textSizeAdjustB->style()->computedFontSize());
}
TEST_F(TextAutosizerTest, DISABLED_PrefixedTextSizeAdjustIsAlias)
TEST_F(TextAutosizerTest, PrefixedTextSizeAdjustIsAlias)
{
setBodyInnerHTML(
"<meta name='viewport' content='width=800'>"
"<style>"
" html { font-size: 16px; }"
" body { width: 800px; margin: 0; overflow-y: hidden; }"
......@@ -247,4 +239,160 @@ TEST_F(TextAutosizerTest, DISABLED_PrefixedTextSizeAdjustIsAlias)
EXPECT_FLOAT_EQ(.5f, textSizeAdjust->style()->getTextSizeAdjust().multiplier());
}
TEST_F(TextAutosizerTest, AccessibilityFontScaleFactor)
{
document().settings()->setAccessibilityFontScaleFactor(1.5);
setBodyInnerHTML(
"<style>"
" html { font-size: 16px; }"
" body { width: 800px; margin: 0; overflow-y: hidden; }"
"</style>"
"<div id='autosized'>"
" Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor"
" incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud"
" exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure"
" dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur."
" Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt"
" mollit anim id est laborum."
"</div>");
Element* autosized = document().getElementById("autosized");
EXPECT_FLOAT_EQ(16.f, autosized->layoutObject()->style()->specifiedFontSize());
// 1.5 * (specified font-size = 16px) * (viewport width = 800px) / (window width = 320px) = 60px.
EXPECT_FLOAT_EQ(60.f, autosized->layoutObject()->style()->computedFontSize());
}
TEST_F(TextAutosizerTest, ChangingAccessibilityFontScaleFactor)
{
document().settings()->setAccessibilityFontScaleFactor(1);
setBodyInnerHTML(
"<style>"
" html { font-size: 16px; }"
" body { width: 800px; margin: 0; overflow-y: hidden; }"
"</style>"
"<div id='autosized'>"
" Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor"
" incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud"
" exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure"
" dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur."
" Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt"
" mollit anim id est laborum."
"</div>");
Element* autosized = document().getElementById("autosized");
EXPECT_FLOAT_EQ(16.f, autosized->layoutObject()->style()->specifiedFontSize());
// 1.0 * (specified font-size = 16px) * (viewport width = 800px) / (window width = 320px) = 40px.
EXPECT_FLOAT_EQ(40.f, autosized->layoutObject()->style()->computedFontSize());
document().settings()->setAccessibilityFontScaleFactor(2);
document().view()->updateAllLifecyclePhases();
EXPECT_FLOAT_EQ(16.f, autosized->layoutObject()->style()->specifiedFontSize());
// 2.0 * (specified font-size = 16px) * (viewport width = 800px) / (window width = 320px) = 80px.
EXPECT_FLOAT_EQ(80.f, autosized->layoutObject()->style()->computedFontSize());
}
TEST_F(TextAutosizerTest, TextSizeAdjustDoesNotDisableAccessibility)
{
document().settings()->setAccessibilityFontScaleFactor(1.5);
setBodyInnerHTML(
"<style>"
" html { font-size: 16px; }"
" body { width: 800px; margin: 0; overflow-y: hidden; }"
"</style>"
"<div id='textSizeAdjustNone' style='text-size-adjust: none;'>"
" Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor"
" incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud"
" exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure"
" dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur."
" Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt"
" mollit anim id est laborum."
"</div>"
"<div id='textSizeAdjustDouble' style='text-size-adjust: 200%;'>"
" Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor"
" incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud"
" exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure"
" dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur."
" Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt"
" mollit anim id est laborum."
"</div>");
Element* textSizeAdjustNone = document().getElementById("textSizeAdjustNone");
EXPECT_FLOAT_EQ(16.f, textSizeAdjustNone->layoutObject()->style()->specifiedFontSize());
// 1.5 * (specified font-size = 16px) = 24px.
EXPECT_FLOAT_EQ(24.f, textSizeAdjustNone->layoutObject()->style()->computedFontSize());
Element* textSizeAdjustDouble = document().getElementById("textSizeAdjustDouble");
EXPECT_FLOAT_EQ(16.f, textSizeAdjustDouble->layoutObject()->style()->specifiedFontSize());
// 1.5 * (specified font-size = 16px) * (text size adjustment = 2) = 48px.
EXPECT_FLOAT_EQ(48.f, textSizeAdjustDouble->layoutObject()->style()->computedFontSize());
// Changing the accessibility font scale factor should change the adjusted size.
document().settings()->setAccessibilityFontScaleFactor(2);
document().view()->updateAllLifecyclePhases();
EXPECT_FLOAT_EQ(16.f, textSizeAdjustNone->layoutObject()->style()->specifiedFontSize());
// 2.0 * (specified font-size = 16px) = 32px.
EXPECT_FLOAT_EQ(32.f, textSizeAdjustNone->layoutObject()->style()->computedFontSize());
EXPECT_FLOAT_EQ(16.f, textSizeAdjustDouble->layoutObject()->style()->specifiedFontSize());
// 2.0 * (specified font-size = 16px) * (text size adjustment = 2) = 64px.
EXPECT_FLOAT_EQ(64.f, textSizeAdjustDouble->layoutObject()->style()->computedFontSize());
}
// https://crbug.com/646237
TEST_F(TextAutosizerTest, DISABLED_TextSizeAdjustWithoutNeedingAutosizing)
{
document().settings()->setTextAutosizingWindowSizeOverride(IntSize(800, 600));
setBodyInnerHTML(
"<style>"
" html { font-size: 16px; }"
" body { width: 800px; margin: 0; overflow-y: hidden; }"
"</style>"
"<div id='textSizeAdjust' style='text-size-adjust: 150%;'>"
" Text"
"</div>");
LayoutObject* textSizeAdjust = document().getElementById("textSizeAdjust")->layoutObject();
EXPECT_FLOAT_EQ(16.f, textSizeAdjust->style()->specifiedFontSize());
EXPECT_FLOAT_EQ(24.f, textSizeAdjust->style()->computedFontSize());
EXPECT_FLOAT_EQ(1.5f, textSizeAdjust->style()->getTextSizeAdjust().multiplier());
}
TEST_F(TextAutosizerTest, DeviceScaleAdjustmentWithViewport)
{
setBodyInnerHTML(
"<meta name='viewport' content='width=800'>"
"<style>"
" html { font-size: 16px; }"
" body { width: 800px; margin: 0; overflow-y: hidden; }"
"</style>"
"<div id='autosized'>"
" Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor"
" incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud"
" exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure"
" dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur."
" Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt"
" mollit anim id est laborum."
"</div>");
document().settings()->setViewportMetaEnabled(true);
document().settings()->setDeviceScaleAdjustment(1.5f);
document().view()->updateAllLifecyclePhases();
Element* autosized = document().getElementById("autosized");
EXPECT_FLOAT_EQ(16.f, autosized->layoutObject()->style()->specifiedFontSize());
// (specified font-size = 16px) * (viewport width = 800px) / (window width = 320px) = 40px.
// The device scale adjustment of 1.5 is ignored.
EXPECT_FLOAT_EQ(40.f, autosized->layoutObject()->style()->computedFontSize());
document().settings()->setViewportMetaEnabled(false);
// TODO(pdr): This should not be needed but changing viewport settings does
// not invalidate TextAutosizing, see ViewportDescriptionChange in Page.cpp.
document().settings()->setDeviceScaleAdjustment(2.0f);
document().view()->updateAllLifecyclePhases();
autosized = document().getElementById("autosized");
EXPECT_FLOAT_EQ(16.f, autosized->layoutObject()->style()->specifiedFontSize());
// (device scale adjustment = 2) * (specified font-size = 16px) * (viewport width = 800px) / (window width = 320px) = 80px.
EXPECT_FLOAT_EQ(80.f, autosized->layoutObject()->style()->computedFontSize());
}
} // namespace blink
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