iBooks text can overlap, sometimes columns are shifted splitting words.
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 29 Apr 2017 21:16:44 +0000 (21:16 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 29 Apr 2017 21:16:44 +0000 (21:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=171472
<rdar://problem/31096037>

Reviewed by Antti Koivisto.

Source/WebCore:

Instead of just checking if the glyph is taller than the line, we need to ensure that both the
ascent and the descent have enough space (this is for -webkit-line-box-contain: glyph).

Test: fast/text/simple-line-layout-glyph-overflows-line.html

* rendering/SimpleLineLayout.cpp:
(WebCore::SimpleLineLayout::canUseForText): compute the available space for the ascent/descent
and check them against the ceil-ed(see FontCascade::floatWidthForSimpleText) glyph min/max y.

LayoutTests:

* fast/text/simple-line-layout-glyph-overflows-line-expected.html: Added.
* fast/text/simple-line-layout-glyph-overflows-line.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@215976 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/text/simple-line-layout-glyph-overflows-line-expected.html [new file with mode: 0644]
LayoutTests/fast/text/simple-line-layout-glyph-overflows-line.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/SimpleLineLayout.cpp

index 374a855..79cb807 100644 (file)
@@ -1,3 +1,14 @@
+2017-04-28  Zalan Bujtas  <zalan@apple.com>
+
+        iBooks text can overlap, sometimes columns are shifted splitting words.
+        https://bugs.webkit.org/show_bug.cgi?id=171472
+        <rdar://problem/31096037>
+
+        Reviewed by Antti Koivisto.
+
+        * fast/text/simple-line-layout-glyph-overflows-line-expected.html: Added.
+        * fast/text/simple-line-layout-glyph-overflows-line.html: Added.
+
 2017-04-28  Per Arne Vollan  <pvollan@apple.com>
 
         Crash under WebCore::AccessibilityRenderObject::handleAriaExpandedChanged().
diff --git a/LayoutTests/fast/text/simple-line-layout-glyph-overflows-line-expected.html b/LayoutTests/fast/text/simple-line-layout-glyph-overflows-line-expected.html
new file mode 100644 (file)
index 0000000..6a78bbd
--- /dev/null
@@ -0,0 +1,28 @@
+<!doctype html>
+<html>
+<head>
+<title>This tests that we bail out of simple line layout when glyph overflows the line</title>
+<style>
+div {
+       -webkit-line-box-contain: block glyphs;
+       font-size: 18px;
+}
+</style>
+<script>
+if (window.internals)
+   internals.settings.setSimpleLineLayoutEnabled(false);
+</script>
+</head>
+<body>
+<div style="line-height: 13px">f<br>overflows this line.</div>
+<div style="line-height: 14px">f<br>does not overflow this line.</div>
+<div style="line-height: 15px">f<br>does not overflow this line.</div>
+<div style="line-height: 16px">f<br>does not overflow this line.</div>
+<div style="line-height: 17px">f<br>does not overflow this line.</div>
+<div style="line-height: 18px">f<br>does not overflow this line.</div>
+<div style="line-height: 20px">g<br>does not overflow this line.</div>
+<div style="line-height: 19px">g<br>overflows this line.</div>
+<div style="line-height: 18px">j<br>overflows this line.</div>
+<div style="line-height: 17px">j<br>overflows this line.</div>
+</body>
+</html>
diff --git a/LayoutTests/fast/text/simple-line-layout-glyph-overflows-line.html b/LayoutTests/fast/text/simple-line-layout-glyph-overflows-line.html
new file mode 100644 (file)
index 0000000..89e1581
--- /dev/null
@@ -0,0 +1,24 @@
+<!doctype html>
+<html>
+<head>
+<title>This tests that we bail out of simple line layout when glyph overflows the line</title>
+<style>
+div {
+       -webkit-line-box-contain: block glyphs;
+       font-size: 18px;
+}
+</style>
+</head>
+<body>
+<div style="line-height: 13px">f<br>overflows this line.</div>
+<div style="line-height: 14px">f<br>does not overflow this line.</div>
+<div style="line-height: 15px">f<br>does not overflow this line.</div>
+<div style="line-height: 16px">f<br>does not overflow this line.</div>
+<div style="line-height: 17px">f<br>does not overflow this line.</div>
+<div style="line-height: 18px">f<br>does not overflow this line.</div>
+<div style="line-height: 20px">g<br>does not overflow this line.</div>
+<div style="line-height: 19px">g<br>overflows this line.</div>
+<div style="line-height: 18px">j<br>overflows this line.</div>
+<div style="line-height: 17px">j<br>overflows this line.</div>
+</body>
+</html>
index 9c6a1d3..80e7957 100644 (file)
@@ -1,3 +1,20 @@
+2017-04-28  Zalan Bujtas  <zalan@apple.com>
+
+        iBooks text can overlap, sometimes columns are shifted splitting words.
+        https://bugs.webkit.org/show_bug.cgi?id=171472
+        <rdar://problem/31096037>
+
+        Reviewed by Antti Koivisto.
+
+        Instead of just checking if the glyph is taller than the line, we need to ensure that both the
+        ascent and the descent have enough space (this is for -webkit-line-box-contain: glyph).
+
+        Test: fast/text/simple-line-layout-glyph-overflows-line.html
+
+        * rendering/SimpleLineLayout.cpp:
+        (WebCore::SimpleLineLayout::canUseForText): compute the available space for the ascent/descent
+        and check them against the ceil-ed(see FontCascade::floatWidthForSimpleText) glyph min/max y.
+
 2017-04-29  Nan Wang  <n_wang@apple.com>
 
         AX: Improve performance of addChildren()/childrenChanged()
index 09c551a..b3f1d0f 100644 (file)
@@ -109,6 +109,15 @@ static AvoidanceReasonFlags canUseForText(const CharacterType* text, unsigned le
 {
     AvoidanceReasonFlags reasons = { };
     auto& primaryFont = fontCascade.primaryFont();
+    auto& fontMetrics = primaryFont.fontMetrics();
+    auto availableSpaceForGlyphAscent = fontMetrics.ascent();
+    auto availableSpaceForGlyphDescent = fontMetrics.descent();
+    if (lineHeightConstraint) {
+        auto lineHeightPadding = *lineHeightConstraint - fontMetrics.height();
+        availableSpaceForGlyphAscent += lineHeightPadding / 2;
+        availableSpaceForGlyphDescent += lineHeightPadding / 2;
+    }
+
     for (unsigned i = 0; i < length; ++i) {
         auto character = text[i];
         if (FontCascade::treatAsSpace(character))
@@ -125,8 +134,11 @@ static AvoidanceReasonFlags canUseForText(const CharacterType* text, unsigned le
         if (!glyphData.isValid() || glyphData.font != &primaryFont)
             SET_REASON_AND_RETURN_IF_NEEDED(FlowPrimaryFontIsInsufficient, reasons, includeReasons);
 
-        if (lineHeightConstraint && primaryFont.boundsForGlyph(glyphData.glyph).height() > *lineHeightConstraint)
-            SET_REASON_AND_RETURN_IF_NEEDED(FlowFontHasOverflowGlyph, reasons, includeReasons);
+        if (lineHeightConstraint) {
+            auto bounds = primaryFont.boundsForGlyph(glyphData.glyph);
+            if (ceilf(-bounds.y()) > availableSpaceForGlyphAscent || ceilf(bounds.maxY()) > availableSpaceForGlyphDescent)
+                SET_REASON_AND_RETURN_IF_NEEDED(FlowFontHasOverflowGlyph, reasons, includeReasons);
+        }
     }
     return reasons;
 }