Diff
Modified: trunk/LayoutTests/ChangeLog (223551 => 223552)
--- trunk/LayoutTests/ChangeLog 2017-10-17 16:34:45 UTC (rev 223551)
+++ trunk/LayoutTests/ChangeLog 2017-10-17 17:04:51 UTC (rev 223552)
@@ -1,3 +1,16 @@
+2017-10-17 Daniel Bates <[email protected]>
+
+ REGRESSION (r222670 and r222732): RTL truncated text may not be drawn
+ https://bugs.webkit.org/show_bug.cgi?id=178278
+ <rdar://problem/34982818>
+
+ Reviewed by Darin Adler.
+
+ Add a test to ensure that we draw right-to-left truncated text correctly.
+
+ * fast/text/ellipsis-text-rtl-expected.html: Added.
+ * fast/text/ellipsis-text-rtl.html: Added.
+
2017-10-10 Yusuke Suzuki <[email protected]>
[JSC] __proto__ getter should be fast
Added: trunk/LayoutTests/fast/text/ellipsis-text-rtl-expected.html (0 => 223552)
--- trunk/LayoutTests/fast/text/ellipsis-text-rtl-expected.html (rev 0)
+++ trunk/LayoutTests/fast/text/ellipsis-text-rtl-expected.html 2017-10-17 17:04:51 UTC (rev 223552)
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+@font-face {
+ font-family: "Ahem";
+ src: url("../../resources/Ahem.ttf");
+}
+
+#expected {
+ font: 20px/1 "Ahem";
+ overflow: hidden;
+}
+</style>
+</head>
+<body>
+<div id="expected">ab</div>
+</body>
+</html>
Added: trunk/LayoutTests/fast/text/ellipsis-text-rtl.html (0 => 223552)
--- trunk/LayoutTests/fast/text/ellipsis-text-rtl.html (rev 0)
+++ trunk/LayoutTests/fast/text/ellipsis-text-rtl.html 2017-10-17 17:04:51 UTC (rev 223552)
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+@font-face {
+ font-family: "Ahem";
+ src: url("../../resources/Ahem.ttf");
+}
+
+#test {
+ font: 20px/1 "Ahem";
+ direction: rtl;
+ unicode-bidi: bidi-override;
+ text-overflow: ellipsis;
+ overflow: hidden;
+ white-space: nowrap;
+ width: 40px;
+}
+</style>
+</head>
+<body>
+<div id="test">abcXYZ</div>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (223551 => 223552)
--- trunk/Source/WebCore/ChangeLog 2017-10-17 16:34:45 UTC (rev 223551)
+++ trunk/Source/WebCore/ChangeLog 2017-10-17 17:04:51 UTC (rev 223552)
@@ -1,3 +1,39 @@
+2017-10-17 Daniel Bates <[email protected]>
+
+ REGRESSION (r222670 and r222732): RTL truncated text may not be drawn
+ https://bugs.webkit.org/show_bug.cgi?id=178278
+ <rdar://problem/34982818>
+
+ Reviewed by Darin Adler.
+
+ Revert r222732 and partially revert r222670. The underlying font rendering machinery implements
+ text truncation by taking a TextRun object that represents all of the text in the line fragment
+ and a subrange of the glyphs to render from this fragment. Only the glyphs in this subrange are
+ drawn and they are drawn in the same position they would be in had the entire line fragment been
+ drawn. Following r222670 InlineTextBox applies the truncation to the TextRun in InlineTextBox::text().
+ Together with r222732, which assumed that the number of glyphs to draw is equal to the length of
+ the TextRun, a truncated text run would be drawn at the wrong position on screen and could give
+ the impression that the text is not drawn. Instead InlineTextBox::text() should always return
+ the text for the entire line fragment without considering truncation and when calling TextPainter::paint()
+ we need to pass the truncated length of the line fragment.
+
+ Test: fast/text/ellipsis-text-rtl.html
+
+ * rendering/InlineTextBox.cpp:
+ (WebCore::InlineTextBox::paint): Compute the truncated length (number of glyphs) and pass this
+ to TextPainter::paint()
+ (WebCore::InlineTextBox::text const): Do not apply truncation to the text run. Truncation is
+ implemented by telling the underlying font rendering machinery to paint the subrange of the
+ text run that represents the non-truncated (visible) text.
+ * rendering/InlineTextBox.h:
+ * rendering/SimpleLineLayoutFunctions.cpp:
+ (WebCore::SimpleLineLayout::paintFlow): Pass the entire length of the text run as we did prior
+ to r222732.
+ * rendering/TextPainter.cpp:
+ (WebCore::TextPainter::paint): Take a length that represents the number of glyphs to draw from
+ the text run as we use to take prior to r222732.
+ * rendering/TextPainter.h:
+
2017-10-17 Zalan Bujtas <[email protected]>
[FrameView::layout cleanup] Move text auto sizing logic to a separate function
Modified: trunk/Source/WebCore/rendering/InlineTextBox.cpp (223551 => 223552)
--- trunk/Source/WebCore/rendering/InlineTextBox.cpp 2017-10-17 16:34:45 UTC (rev 223551)
+++ trunk/Source/WebCore/rendering/InlineTextBox.cpp 2017-10-17 17:04:51 UTC (rev 223552)
@@ -491,6 +491,8 @@
auto text = this->text();
TextRun textRun = createTextRun(text);
unsigned length = textRun.length();
+ if (m_truncation != cNoTruncation)
+ length = m_truncation;
unsigned selectionStart = 0;
unsigned selectionEnd = 0;
@@ -549,7 +551,7 @@
if (currentEnd < length)
textPainter.paintRange(textRun, boxRect, textOrigin, currentEnd, length);
} else
- textPainter.paint(textRun, boxRect, textOrigin, selectionStart, selectionEnd, paintSelectedTextOnly, paintSelectedTextSeparately, paintNonSelectedTextOnly);
+ textPainter.paint(textRun, length, boxRect, textOrigin, selectionStart, selectionEnd, paintSelectedTextOnly, paintSelectedTextSeparately, paintNonSelectedTextOnly);
// Paint decorations
TextDecoration textDecorations = lineStyle.textDecorationsInEffect();
@@ -1077,8 +1079,6 @@
String InlineTextBox::text(bool ignoreCombinedText, bool ignoreHyphen) const
{
- if (UNLIKELY(m_truncation == cFullTruncation))
- return emptyString();
if (auto* combinedText = this->combinedText()) {
if (ignoreCombinedText)
return renderer().text()->substring(m_start, m_len);
@@ -1089,7 +1089,7 @@
return renderer().text()->substring(m_start, m_len);
return makeString(StringView(renderer().text()).substring(m_start, m_len), lineStyle().hyphenString());
}
- return renderer().text()->substring(m_start, m_truncation != cNoTruncation ? m_truncation : m_len);
+ return renderer().text()->substring(m_start, m_len);
}
inline const RenderCombineText* InlineTextBox::combinedText() const
Modified: trunk/Source/WebCore/rendering/InlineTextBox.h (223551 => 223552)
--- trunk/Source/WebCore/rendering/InlineTextBox.h 2017-10-17 16:34:45 UTC (rev 223551)
+++ trunk/Source/WebCore/rendering/InlineTextBox.h 2017-10-17 17:04:51 UTC (rev 223552)
@@ -167,7 +167,7 @@
const RenderCombineText* combinedText() const;
const FontCascade& lineFont() const;
- String text(bool ignoreCombinedText = false, bool ignoreHyphen = false) const; // The text to render.
+ String text(bool ignoreCombinedText = false, bool ignoreHyphen = false) const; // The effective text for the run.
TextRun createTextRun(String&) const;
ExpansionBehavior expansionBehavior() const;
Modified: trunk/Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp (223551 => 223552)
--- trunk/Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp 2017-10-17 16:34:45 UTC (rev 223551)
+++ trunk/Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp 2017-10-17 17:04:51 UTC (rev 223552)
@@ -120,7 +120,7 @@
TextRun textRun(run.hasHyphen() ? textWithHyphen : run.text(), 0, run.expansion(), run.expansionBehavior());
textRun.setTabSize(!style.collapseWhiteSpace(), style.tabSize());
FloatPoint textOrigin = FloatPoint(rect.x() + paintOffset.x(), roundToDevicePixel(run.baselinePosition() + paintOffset.y(), deviceScaleFactor));
- textPainter.paint(textRun, rect, textOrigin);
+ textPainter.paint(textRun, textRun.length(), rect, textOrigin);
if (textDecorationPainter) {
textDecorationPainter->setWidth(rect.width());
textDecorationPainter->paintTextDecoration(textRun, textOrigin, rect.location() + paintOffset);
Modified: trunk/Source/WebCore/rendering/TextPainter.cpp (223551 => 223552)
--- trunk/Source/WebCore/rendering/TextPainter.cpp 2017-10-17 16:34:45 UTC (rev 223551)
+++ trunk/Source/WebCore/rendering/TextPainter.cpp 2017-10-17 17:04:51 UTC (rev 223552)
@@ -190,11 +190,11 @@
paintTextAndEmphasisMarksIfNeeded(textRun, boxRect, textOrigin, start, end, m_style, m_shadow);
}
-void TextPainter::paint(const TextRun& textRun, const FloatRect& boxRect, const FloatPoint& textOrigin, unsigned selectionStart, unsigned selectionEnd,
+void TextPainter::paint(const TextRun& textRun, unsigned length, const FloatRect& boxRect, const FloatPoint& textOrigin, unsigned selectionStart, unsigned selectionEnd,
bool paintSelectedTextOnly, bool paintSelectedTextSeparately, bool paintNonSelectedTextOnly)
{
ASSERT(m_font);
- unsigned length = textRun.length();
+ ASSERT(length <= textRun.length());
if (!paintSelectedTextOnly) {
// For stroked painting, we have to change the text drawing mode. It's probably dangerous to leave that mutated as a side
// effect, so only when we know we're stroking, do a save/restore.
Modified: trunk/Source/WebCore/rendering/TextPainter.h (223551 => 223552)
--- trunk/Source/WebCore/rendering/TextPainter.h 2017-10-17 16:34:45 UTC (rev 223551)
+++ trunk/Source/WebCore/rendering/TextPainter.h 2017-10-17 17:04:51 UTC (rev 223552)
@@ -62,7 +62,7 @@
void setEmphasisMark(const AtomicString& mark, float offset, const RenderCombineText*);
void paintRange(const TextRun&, const FloatRect& boxRect, const FloatPoint& textOrigin, unsigned start, unsigned end);
- void paint(const TextRun&, const FloatRect& boxRect, const FloatPoint& textOrigin, unsigned selectionStart = 0, unsigned selectionEnd = 0, bool paintSelectedTextOnly = false, bool paintSelectedTextSeparately = false, bool paintNonSelectedTextOnly = false);
+ void paint(const TextRun&, unsigned length, const FloatRect& boxRect, const FloatPoint& textOrigin, unsigned selectionStart = 0, unsigned selectionEnd = 0, bool paintSelectedTextOnly = false, bool paintSelectedTextSeparately = false, bool paintNonSelectedTextOnly = false);
private:
void paintTextOrEmphasisMarks(const FontCascade&, const TextRun&, const AtomicString& emphasisMark, float emphasisMarkOffset,