Modified: trunk/Source/WebCore/ChangeLog (223258 => 223259)
--- trunk/Source/WebCore/ChangeLog 2017-10-12 21:29:29 UTC (rev 223258)
+++ trunk/Source/WebCore/ChangeLog 2017-10-12 21:31:36 UTC (rev 223259)
@@ -1,3 +1,55 @@
+2017-10-12 Daniel Bates <[email protected]>
+
+ Teach InlineTextBox::clampOffset() about combined text and hyphenation
+ https://bugs.webkit.org/show_bug.cgi?id=178032
+
+ Reviewed by Zalan Bujtas.
+
+ Treat combined text and the last character of a word halve plus hyphen as single units.
+
+ With regards to combined text, ideally we would allow arbitrary selection inside combined
+ text. Currently we do not support selection of combined text. To simplify the process of
+ adding support for selecting combined text we treat combined text as a single unit. Once
+ we are confident that we correctly implemented such support we can re-evaluate allowing
+ arbitrary selection of combined text.
+
+ With regards to treating the last character of a word halve plus hyphen as a single unit.
+ This patch extends the targeted fix made for document markers in r223013 to all code that
+ makes use of clamped offsets as a result the selection rect for inline boxes more accurately
+ reflect the rectangle(s) that make up the painted selection. This is a step towards reconciling
+ the difference between the computation of the rectangle that represents an arbitrary
+ selection and the code that paints the active selection as part of <https://bugs.webkit.org/show_bug.cgi?id=138913>.
+
+ * rendering/InlineTextBox.cpp:
+ (WebCore::InlineTextBox::localSelectionRect const): Compute text run, including combined text
+ or hyphens due to line wrapping now that specified start and end positions are clamped with
+ respect to combined text and hyphens (computed earlier in this function). Only measure the
+ text represented by the selection if the start position > 0 or the end position is not equal
+ to the length of the run.
+ (WebCore::InlineTextBox::paint): Remove unnecessary code to fix up the selection start and
+ end positions based on the truncation offset as this is done by clampedOffset(), called by
+ selectionStartEnd().
+ (WebCore::InlineTextBox::clampedOffset const): Modified to adjust the clamped offset with
+ respect to truncation as well as treat combined text or a trailing word halve plus hyphen
+ as single units. Assert that we are not fully truncated because it does not make sense to
+ be computing the clamped offset in such a situation since nothing should be painted.
+ (WebCore::InlineTextBox::selectionStartEnd const): Modified to compute the end of an inside
+ selection using clampedOffset() to account for truncation, combined text or a hyphen. We
+ already are using clampedOffset() when computing the start and end position for all other
+ selection states.
+ (WebCore::InlineTextBox::paintSelection): Compute text run, including combined text
+ or hyphens due to line wrapping now that specified start and end positions are clamped with
+ respect to combined text and hyphens (computed earlier in this function). Remove unnecessary
+ code to adjust selection end point with respect to truncation, combined text, or an added
+ hyphen now that selectionStartEnd() takes care of this (via clampedOffset()).
+ (WebCore::InlineTextBox::paintTextSubrangeBackground): Compute text run, including combined
+ text or hyphens due to line wrapping now that specified start and end positions are clamped
+ with respect to combined text and hyphens (computed earlier in this function).
+ (WebCore::InlineTextBox::paintDocumentMarker): Compute text run, including combined text now
+ that specified start and end positions are clamped with respect to combined text (computed earlier in this function).
+ Also remove unnecessary code to adjust end offset of the marker with respect to truncation
+ and length of the text run as clampedOffset() now does this for us.
+
2017-10-11 Simon Fraser <[email protected]>
Don't assert if mix-blend-mode is set to a non-separable blend mode on a composited layer
Modified: trunk/Source/WebCore/rendering/InlineTextBox.cpp (223258 => 223259)
--- trunk/Source/WebCore/rendering/InlineTextBox.cpp 2017-10-12 21:29:29 UTC (rev 223258)
+++ trunk/Source/WebCore/rendering/InlineTextBox.cpp 2017-10-12 21:31:36 UTC (rev 223259)
@@ -197,16 +197,12 @@
LayoutUnit selectionTop = this->selectionTop();
LayoutUnit selectionHeight = this->selectionHeight();
- // FIXME: Adjust selection rect with respect to combined text.
- bool ignoreCombinedText = true;
- auto text = this->text(ignoreCombinedText);
+ auto text = this->text();
TextRun textRun = createTextRun(text);
- // FIXME: Shouldn't we adjust ePos to textRun.length() if ePos == (m_truncation != cNoTruncation ? m_truncation : m_len)
- // so that the selection spans the hypen/combined text?
LayoutRect selectionRect = LayoutRect(LayoutPoint(logicalLeft(), selectionTop), LayoutSize(m_logicalWidth, selectionHeight));
// Avoid computing the font width when the entire line box is selected as an optimization.
- if (sPos || ePos != m_len)
+ if (sPos || ePos != textRun.length())
lineFont().adjustSelectionRectForText(textRun, selectionRect, sPos, ePos);
// FIXME: The computation of the snapped selection rect differs from the computation of this rect
// in paintSelection(). See <https://bugs.webkit.org/show_bug.cgi?id=138913>.
@@ -501,11 +497,6 @@
if (haveSelection && (paintSelectedTextOnly || paintSelectedTextSeparately))
std::tie(selectionStart, selectionEnd) = selectionStartEnd();
- if (m_truncation != cNoTruncation) {
- selectionStart = std::min(selectionStart, static_cast<unsigned>(m_truncation));
- selectionEnd = std::min(selectionEnd, static_cast<unsigned>(m_truncation));
- }
-
float emphasisMarkOffset = 0;
bool emphasisMarkAbove;
bool hasTextEmphasis = emphasisMarkExistsAndIsAbove(lineStyle, emphasisMarkAbove);
@@ -598,7 +589,20 @@
unsigned InlineTextBox::clampedOffset(unsigned x) const
{
- return std::max(std::min(x, start() + len()), start()) - start();
+ ASSERT(m_truncation != cFullTruncation);
+ unsigned offset = std::max(std::min(x, m_start + m_len), m_start) - m_start;
+ if (m_truncation != cNoTruncation)
+ offset = std::min<unsigned>(offset, m_truncation);
+ else if (offset == m_len) {
+ // Fix up the offset if we are combined text or have a hyphen because we manage these embellishments.
+ // That is, they are not reflected in renderer().text(). We treat combined text as a single unit.
+ // We also treat the last codepoint in this box and the hyphen as a single unit.
+ if (auto* combinedText = this->combinedText())
+ offset = combinedText->combinedStringForRendering().length();
+ else if (hasHyphen())
+ offset += lineStyle().hyphenString().length();
+ }
+ return offset;
}
std::pair<unsigned, unsigned> InlineTextBox::selectionStartEnd() const
@@ -605,7 +609,7 @@
{
auto selectionState = renderer().selectionState();
if (selectionState == RenderObject::SelectionInside)
- return { 0, m_len };
+ return { 0, clampedOffset(m_start + m_len) };
auto start = renderer().view().selection().startPosition();
auto end = renderer().view().selection().endPosition();
@@ -643,14 +647,8 @@
// Note that if the text is truncated, we let the thing being painted in the truncation
// draw its own highlight.
-
- // FIXME: Adjust text run for combined text.
- bool ignoreCombinedText = true;
- auto text = this->text(ignoreCombinedText);
+ auto text = this->text();
TextRun textRun = createTextRun(text);
- unsigned endOfLineIgnoringHyphenAndCombinedText = m_truncation != cNoTruncation ? m_truncation : m_len;
- if (selectionEnd == endOfLineIgnoringHyphenAndCombinedText)
- selectionEnd = textRun.length(); // Extend selection to include hyphen/combined text.
const RootInlineBox& rootBox = root();
LayoutUnit selectionBottom = rootBox.selectionBottom();
@@ -661,6 +659,8 @@
LayoutRect selectionRect = LayoutRect(boxOrigin.x(), boxOrigin.y() - deltaY, m_logicalWidth, selectionHeight);
lineFont().adjustSelectionRectForText(textRun, selectionRect, selectionStart, selectionEnd);
+
+ // FIXME: Support painting combined text.
context.fillRect(snapRectToDevicePixelsWithWritingDirection(selectionRect, renderer().document().deviceScaleFactor(), textRun.ltr()), c);
#else
UNUSED_PARAM(context);
@@ -684,12 +684,11 @@
LayoutUnit deltaY = renderer().style().isFlippedLinesWritingMode() ? selectionBottom() - logicalBottom() : logicalTop() - selectionTop();
LayoutRect selectionRect = LayoutRect(boxOrigin.x(), boxOrigin.y() - deltaY, 0, selectionHeight());
- // FIXME: Adjust text run for combined text and hyphenation.
- bool ignoreCombinedText = true;
- bool ignoreHyphen = true;
- auto text = this->text(ignoreCombinedText, ignoreHyphen);
+ auto text = this->text();
TextRun textRun = createTextRun(text);
lineFont().adjustSelectionRectForText(textRun, selectionRect, startOffset, endOffset);
+
+ // FIXME: Support painting combined text.
context.fillRect(snapRectToDevicePixelsWithWritingDirection(selectionRect, renderer().document().deviceScaleFactor(), textRun.ltr()), color);
}
@@ -781,20 +780,15 @@
unsigned startPosition = clampedOffset(subrange.startOffset);
unsigned endPosition = clampedOffset(subrange.endOffset);
- if (m_truncation != cNoTruncation)
- endPosition = std::min(endPosition, static_cast<unsigned>(m_truncation));
-
// Calculate start & width
- // FIXME: Adjust text run for combined text.
- bool ignoreCombinedText = true;
int deltaY = renderer().style().isFlippedLinesWritingMode() ? selectionBottom() - logicalBottom() : logicalTop() - selectionTop();
int selHeight = selectionHeight();
FloatPoint startPoint(boxOrigin.x(), boxOrigin.y() - deltaY);
- auto text = this->text(ignoreCombinedText);
+ auto text = this->text();
TextRun run = createTextRun(text);
LayoutRect selectionRect = LayoutRect(startPoint, FloatSize(0, selHeight));
- lineFont().adjustSelectionRectForText(run, selectionRect, startPosition, endPosition >= len() ? run.length() : endPosition);
+ lineFont().adjustSelectionRectForText(run, selectionRect, startPosition, endPosition);
IntRect markerRect = enclosingIntRect(selectionRect);
start = markerRect.x() - startPoint.x();
width = markerRect.width();
@@ -838,6 +832,7 @@
// In larger fonts, though, place the underline up near the baseline to prevent a big gap.
underlineOffset = baseline + 2;
}
+ // FIXME: Support painting combined text.
context.drawLineForDocumentMarker(FloatPoint(boxOrigin.x() + start, boxOrigin.y() + underlineOffset), width, lineStyleForSubrangeType(subrange.type));
}
