[Text autosizing] [iPadOS] Product label text is clipped in portrait mode on the front page of sephora.com
https://bugs.webkit.org/show_bug.cgi?id=199806
<rdar://problem/52902482>
Reviewed by Zalan Bujtas.
Source/WebCore:
On sephora.com, some product label text is currently boosted by idempotent text autosizing, which causes the
labels be vertically clipped. This patch augments the idempotent text autosizing heuristic to avoid this case by
checking if the element to be boosted has a fixed height or max height, whose value is very close to a small
integer multiple of the line height. In this case, it's likely that the website expects the text to be no more
than a few lines' worth of height, so boosting the text is likely to break the page.
Test: fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates.html
* rendering/style/RenderStyle.cpp:
(WebCore::RenderStyle::isIdempotentTextAutosizingCandidate const):
* rendering/style/TextSizeAdjustment.cpp:
(WebCore::AutosizeStatus::updateStatus):
* rendering/style/TextSizeAdjustment.h:
Rename Fields::DisplayNone to Fields::AvoidSubtree to avoid introducing another bit in RenderStyle's inherited
flags.
LayoutTests:
Add a new test case to an existing layout test, which mimics the product label text on sephora.com's front page.
* fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates-expected.txt:
* fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates.html:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@247467 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index df7c161..9f4e7db 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2019-07-15 Wenson Hsieh <wenson_hsieh@apple.com>
+
+ [Text autosizing] [iPadOS] Product label text is clipped in portrait mode on the front page of sephora.com
+ https://bugs.webkit.org/show_bug.cgi?id=199806
+ <rdar://problem/52902482>
+
+ Reviewed by Zalan Bujtas.
+
+ Add a new test case to an existing layout test, which mimics the product label text on sephora.com's front page.
+
+ * fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates-expected.txt:
+ * fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates.html:
+
2019-07-11 Myles C. Maxfield <mmaxfield@apple.com>
New York font erroneously gets synthetic bold
diff --git a/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates-expected.txt b/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates-expected.txt
index 6207278..464d944 100644
--- a/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates-expected.txt
+++ b/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates-expected.txt
@@ -32,6 +32,8 @@
PASS result is >= 13
Checking target17:
PASS result is 12
+Checking target18:
+PASS result is 12
PASS successfullyParsed is true
TEST COMPLETE
@@ -51,3 +53,6 @@
Test
Test
Test
+Test
+Test
+Test
diff --git a/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates.html b/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates.html
index 90ae44a..8c588ef 100644
--- a/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates.html
+++ b/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates.html
@@ -28,6 +28,13 @@
<div style="background: green;"><span id="target15" style="font-size: 12px; overflow-y: hidden; float: right;">Test</span></div>
<div style="background: green;"><span id="target16" style="font-size: 12px; float: right;">Test</span></div>
<div style="background: green;"><span id="target17" style="font-size: 12px; -webkit-text-size-adjust: 100%;">Test</span></div>
+<div style="background: green; line-height: 1.25; max-height: 45px; font-size: 12px;">
+ <span>Test</span>
+ <br>
+ <span id="target18">Test</span>
+ <br>
+ <span>Test</span>
+</div>
<script>
let result;
function check(name, shouldGetAutosized) {
@@ -69,6 +76,7 @@
check("target15", true);
check("target16", true);
check("target17", false);
+check("target18", false);
</script>
<script src="../../../../resources/js-test-post.js"></script>
</body>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 37eb615..3851112 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,28 @@
+2019-07-15 Wenson Hsieh <wenson_hsieh@apple.com>
+
+ [Text autosizing] [iPadOS] Product label text is clipped in portrait mode on the front page of sephora.com
+ https://bugs.webkit.org/show_bug.cgi?id=199806
+ <rdar://problem/52902482>
+
+ Reviewed by Zalan Bujtas.
+
+ On sephora.com, some product label text is currently boosted by idempotent text autosizing, which causes the
+ labels be vertically clipped. This patch augments the idempotent text autosizing heuristic to avoid this case by
+ checking if the element to be boosted has a fixed height or max height, whose value is very close to a small
+ integer multiple of the line height. In this case, it's likely that the website expects the text to be no more
+ than a few lines' worth of height, so boosting the text is likely to break the page.
+
+ Test: fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates.html
+
+ * rendering/style/RenderStyle.cpp:
+ (WebCore::RenderStyle::isIdempotentTextAutosizingCandidate const):
+ * rendering/style/TextSizeAdjustment.cpp:
+ (WebCore::AutosizeStatus::updateStatus):
+ * rendering/style/TextSizeAdjustment.h:
+
+ Rename Fields::DisplayNone to Fields::AvoidSubtree to avoid introducing another bit in RenderStyle's inherited
+ flags.
+
2019-07-15 Myles C. Maxfield <mmaxfield@apple.com>
New York font erroneously gets synthetic bold
diff --git a/Source/WebCore/rendering/style/RenderStyle.cpp b/Source/WebCore/rendering/style/RenderStyle.cpp
index 2d2e83a..04c977b 100644
--- a/Source/WebCore/rendering/style/RenderStyle.cpp
+++ b/Source/WebCore/rendering/style/RenderStyle.cpp
@@ -496,7 +496,7 @@
{
// Refer to <rdar://problem/51826266> for more information regarding how this function was generated.
auto fields = OptionSet<AutosizeStatus::Fields>::fromRaw(m_inheritedFlags.autosizeStatus);
- if (fields.contains(AutosizeStatus::Fields::DisplayNone))
+ if (fields.contains(AutosizeStatus::Fields::AvoidSubtree))
return false;
if (fields.contains(AutosizeStatus::Fields::FixedHeight)) {
diff --git a/Source/WebCore/rendering/style/TextSizeAdjustment.cpp b/Source/WebCore/rendering/style/TextSizeAdjustment.cpp
index bd2130e..0528027 100644
--- a/Source/WebCore/rendering/style/TextSizeAdjustment.cpp
+++ b/Source/WebCore/rendering/style/TextSizeAdjustment.cpp
@@ -46,8 +46,36 @@
{
auto result = style.autosizeStatus().fields();
- if (style.display() == DisplayType::None)
- result.add(Fields::DisplayNone);
+ auto shouldAvoidAutosizingEntireSubtree = [&] {
+ if (style.display() == DisplayType::None)
+ return true;
+
+ if (!style.lineHeight().isSpecified() || style.whiteSpace() == WhiteSpace::NoWrap)
+ return false;
+
+ Optional<Length> heightOrMaxHeightAsLength;
+ if (style.height().isFixed() && style.maxHeight().isAuto())
+ heightOrMaxHeightAsLength = style.height();
+ else if (style.maxHeight().isFixed())
+ heightOrMaxHeightAsLength = style.maxHeight();
+
+ if (!heightOrMaxHeightAsLength)
+ return false;
+
+ float heightOrMaxHeight = heightOrMaxHeightAsLength->value();
+ float computedLineHeight = style.computedLineHeight();
+ if (computedLineHeight <= 0)
+ return false;
+
+ float approximateNumberOfLines = heightOrMaxHeight / computedLineHeight;
+ const int maximumNumberOfLines = 5;
+ const float thresholdForConsideringAnApproximateNumberOfLinesToBeCloseToAnInteger = 0.01;
+ return approximateNumberOfLines <= maximumNumberOfLines + thresholdForConsideringAnApproximateNumberOfLinesToBeCloseToAnInteger
+ && approximateNumberOfLines - std::floor(approximateNumberOfLines) <= thresholdForConsideringAnApproximateNumberOfLinesToBeCloseToAnInteger;
+ };
+
+ if (shouldAvoidAutosizingEntireSubtree())
+ result.add(Fields::AvoidSubtree);
if (style.height().isFixed())
result.add(Fields::FixedHeight);
diff --git a/Source/WebCore/rendering/style/TextSizeAdjustment.h b/Source/WebCore/rendering/style/TextSizeAdjustment.h
index 55f2f62..552ca41 100644
--- a/Source/WebCore/rendering/style/TextSizeAdjustment.h
+++ b/Source/WebCore/rendering/style/TextSizeAdjustment.h
@@ -52,7 +52,7 @@
class AutosizeStatus {
public:
enum class Fields : uint8_t {
- DisplayNone = 1 << 0,
+ AvoidSubtree = 1 << 0,
FixedHeight = 1 << 1,
FixedWidth = 1 << 2,
Floating = 1 << 3,
