InlineTextBox::end() should return first-past-end offset
https://bugs.webkit.org/show_bug.cgi?id=201181
Reviewed by Zalan Bujtas.
It currently points to the last character, except for empty text boxes.
This is awkward in itself and also inconsistent, as we use first-past-end offset everywhere else.
* dom/Position.cpp:
(WebCore::Position::downstream const):
Add a check for zero length case to avoid changing behavior.
* layout/Verification.cpp:
(WebCore::Layout::checkForMatchingTextRuns):
(WebCore::Layout::outputMismatchingComplexLineInformationIfNeeded):
* rendering/InlineFlowBox.cpp:
(WebCore::InlineFlowBox::placeBoxRangeInInlineDirection):
* rendering/InlineTextBox.cpp:
(WebCore::InlineTextBox::paint):
(WebCore::InlineTextBox::calculateDocumentMarkerBounds const):
(WebCore::InlineTextBox::collectMarkedTextsForDocumentMarkers const):
(WebCore::InlineTextBox::paintCompositionUnderlines const):
(WebCore::InlineTextBox::paintCompositionUnderline const):
* rendering/InlineTextBox.h:
(WebCore::InlineTextBox::end const):
end = start + len
* rendering/RenderText.cpp:
(WebCore::RenderText::setTextWithOffset):
* rendering/RenderTextLineBoxes.cpp:
(WebCore::localQuadForTextBox):
(WebCore::RenderTextLineBoxes::absoluteRectsForRange const):
(WebCore::RenderTextLineBoxes::absoluteQuadsForRange const):
(WebCore::RenderTextLineBoxes::dirtyRange):
Here the incoming 'end' used linebox style too, move that to the new definition too.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@249160 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index a3ea61c..33d3be6 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,44 @@
+2019-08-27 Antti Koivisto <antti@apple.com>
+
+ InlineTextBox::end() should return first-past-end offset
+ https://bugs.webkit.org/show_bug.cgi?id=201181
+
+ Reviewed by Zalan Bujtas.
+
+ It currently points to the last character, except for empty text boxes.
+ This is awkward in itself and also inconsistent, as we use first-past-end offset everywhere else.
+
+ * dom/Position.cpp:
+ (WebCore::Position::downstream const):
+
+ Add a check for zero length case to avoid changing behavior.
+
+ * layout/Verification.cpp:
+ (WebCore::Layout::checkForMatchingTextRuns):
+ (WebCore::Layout::outputMismatchingComplexLineInformationIfNeeded):
+ * rendering/InlineFlowBox.cpp:
+ (WebCore::InlineFlowBox::placeBoxRangeInInlineDirection):
+ * rendering/InlineTextBox.cpp:
+ (WebCore::InlineTextBox::paint):
+ (WebCore::InlineTextBox::calculateDocumentMarkerBounds const):
+ (WebCore::InlineTextBox::collectMarkedTextsForDocumentMarkers const):
+ (WebCore::InlineTextBox::paintCompositionUnderlines const):
+ (WebCore::InlineTextBox::paintCompositionUnderline const):
+ * rendering/InlineTextBox.h:
+ (WebCore::InlineTextBox::end const):
+
+ end = start + len
+
+ * rendering/RenderText.cpp:
+ (WebCore::RenderText::setTextWithOffset):
+ * rendering/RenderTextLineBoxes.cpp:
+ (WebCore::localQuadForTextBox):
+ (WebCore::RenderTextLineBoxes::absoluteRectsForRange const):
+ (WebCore::RenderTextLineBoxes::absoluteQuadsForRange const):
+ (WebCore::RenderTextLineBoxes::dirtyRange):
+
+ Here the incoming 'end' used linebox style too, move that to the new definition too.
+
2019-08-27 Chris Dumez <cdumez@apple.com>
Crash under WebCore::jsNotificationConstructorPermission
diff --git a/Source/WebCore/dom/Position.cpp b/Source/WebCore/dom/Position.cpp
index 8b05bc8..6d584ed 100644
--- a/Source/WebCore/dom/Position.cpp
+++ b/Source/WebCore/dom/Position.cpp
@@ -873,7 +873,10 @@
unsigned textOffset = currentPosition.offsetInLeafNode();
auto lastTextBox = textRenderer.lastTextBox();
for (auto* box = textRenderer.firstTextBox(); box; box = box->nextTextBox()) {
- if (textOffset <= box->end()) {
+ if (!box->len() && textOffset == box->start())
+ return currentPosition;
+
+ if (textOffset < box->end()) {
if (textOffset >= box->start())
return currentPosition;
continue;
diff --git a/Source/WebCore/layout/Verification.cpp b/Source/WebCore/layout/Verification.cpp
index ccf46d8..3323b12 100644
--- a/Source/WebCore/layout/Verification.cpp
+++ b/Source/WebCore/layout/Verification.cpp
@@ -120,7 +120,7 @@
&& areEssentiallyEqual(inlineTextBox.logicalTop(), inlineRun.logicalTop())
&& areEssentiallyEqual(inlineTextBox.logicalBottom(), inlineRun.logicalBottom())
&& inlineTextBox.start() == inlineRun.textContext()->start()
- && (inlineTextBox.end() + 1) == inlineRun.textContext()->end();
+ && inlineTextBox.end() == inlineRun.textContext()->end();
}
static void collectFlowBoxSubtree(const InlineFlowBox& flowbox, Vector<WebCore::InlineBox*>& inlineBoxes)
@@ -184,7 +184,7 @@
stream << "Mismatching: run";
if (inlineTextBox)
- stream << " (" << inlineTextBox->start() << ", " << inlineTextBox->end() + 1 << ")";
+ stream << " (" << inlineTextBox->start() << ", " << inlineTextBox->end() << ")";
stream << " (" << inlineBox->logicalLeft() << ", " << inlineBox->logicalTop() << ") (" << inlineBox->logicalWidth() << "x" << inlineBox->logicalHeight() << ")";
stream << " inline run";
diff --git a/Source/WebCore/rendering/InlineFlowBox.cpp b/Source/WebCore/rendering/InlineFlowBox.cpp
index 2a1bc289..96a249f 100644
--- a/Source/WebCore/rendering/InlineFlowBox.cpp
+++ b/Source/WebCore/rendering/InlineFlowBox.cpp
@@ -392,7 +392,7 @@
if (renderText.text().length()) {
if (needsWordSpacing && isSpaceOrNewline(renderText.characterAt(textBox.start())))
logicalLeft += textBox.lineStyle().fontCascade().wordSpacing();
- needsWordSpacing = !isSpaceOrNewline(renderText.characterAt(textBox.end()));
+ needsWordSpacing = !isSpaceOrNewline(renderText.characterAt(textBox.end() - 1));
}
textBox.setLogicalLeft(logicalLeft);
if (knownToHaveNoOverflow())
diff --git a/Source/WebCore/rendering/InlineTextBox.cpp b/Source/WebCore/rendering/InlineTextBox.cpp
index dff8b11..90aeacf 100644
--- a/Source/WebCore/rendering/InlineTextBox.cpp
+++ b/Source/WebCore/rendering/InlineTextBox.cpp
@@ -554,7 +554,7 @@
Vector<MarkedText> markedTexts;
if (paintInfo.phase != PaintPhase::Selection) {
// The marked texts for the gaps between document markers and selection are implicitly created by subdividing the entire line.
- markedTexts.append({ clampedOffset(m_start), clampedOffset(end() + 1), MarkedText::Unmarked });
+ markedTexts.append({ clampedOffset(m_start), clampedOffset(end()), MarkedText::Unmarked });
if (!isPrinting) {
markedTexts.appendVector(collectMarkedTextsForDocumentMarkers(TextPaintPhase::Foreground));
@@ -697,7 +697,7 @@
auto height = 0.13247 * fontSize;
// Avoid measuring the text when the entire line box is selected as an optimization.
- if (markedText.startOffset || markedText.endOffset != clampedOffset(end() + 1)) {
+ if (markedText.startOffset || markedText.endOffset != clampedOffset(end())) {
TextRun run = createTextRun();
LayoutRect selectionRect = LayoutRect(0, y, 0, height);
lineFont().adjustSelectionRectForText(run, selectionRect, markedText.startOffset, markedText.endOffset);
@@ -932,7 +932,7 @@
continue;
}
- if (marker->startOffset() > end()) {
+ if (marker->startOffset() >= end()) {
// Marker is completely after this run, bail. A later run will paint it.
break;
}
@@ -1138,13 +1138,13 @@
continue;
}
- if (underline.startOffset > end())
+ if (underline.startOffset >= end())
break; // Underline is completely after this run, bail. A later run will paint it.
// Underline intersects this run. Paint it.
paintCompositionUnderline(paintInfo, boxOrigin, underline);
- if (underline.endOffset > end() + 1)
+ if (underline.endOffset > end())
break; // Underline also runs into the next run. Bail now, no more marker advancement.
}
}
@@ -1165,7 +1165,7 @@
float width = logicalWidth(); // how much line to draw
bool useWholeWidth = true;
unsigned paintStart = m_start;
- unsigned paintEnd = end() + 1; // end points at the last char, not past it
+ unsigned paintEnd = end();
if (paintStart <= underline.startOffset) {
paintStart = underline.startOffset;
useWholeWidth = false;
diff --git a/Source/WebCore/rendering/InlineTextBox.h b/Source/WebCore/rendering/InlineTextBox.h
index 52348a5..5ca2bc8 100644
--- a/Source/WebCore/rendering/InlineTextBox.h
+++ b/Source/WebCore/rendering/InlineTextBox.h
@@ -66,7 +66,7 @@
// FIXME: These accessors should ASSERT(!isDirty()). See https://bugs.webkit.org/show_bug.cgi?id=97264
// Note len() == 1 for combined text regardless of whether the composition is empty. Use hasTextContent() to
unsigned start() const { return m_start; }
- unsigned end() const { return m_len ? m_start + m_len - 1 : m_start; }
+ unsigned end() const { return m_start + m_len; }
unsigned len() const { return m_len; }
void setStart(unsigned start) { m_start = start; }
diff --git a/Source/WebCore/rendering/RenderText.cpp b/Source/WebCore/rendering/RenderText.cpp
index b6305de..6b432f3 100644
--- a/Source/WebCore/rendering/RenderText.cpp
+++ b/Source/WebCore/rendering/RenderText.cpp
@@ -360,11 +360,10 @@
for (InlineTextBox* box = firstTextBox(); box; box = box->nextTextBox()) {
LayoutRect rect;
- // Note, box->end() returns the index of the last character, not the index past it.
- if (start <= box->start() && box->end() < end)
+ if (start <= box->start() && box->end() <= end)
rect = box->localSelectionRect(start, end);
else {
- unsigned realEnd = std::min(box->end() + 1, end);
+ unsigned realEnd = std::min(box->end(), end);
rect = box->localSelectionRect(start, realEnd);
if (rect.isEmpty())
continue;
@@ -398,8 +397,8 @@
if (containingBlock->isRubyBase() || containingBlock->isRubyText())
isLastOnLine = !containingBlock->containingBlock()->inlineBoxWrapper()->nextOnLineExists();
- bool containsStart = box->start() <= start && box->end() + 1 >= start;
- bool containsEnd = box->start() <= end && box->end() + 1 >= end;
+ bool containsStart = box->start() <= start && box->end() >= start;
+ bool containsEnd = box->start() <= end && box->end() >= end;
bool isFixed = false;
IntRect absRect = localToAbsoluteQuad(FloatRect(rect), UseTransforms, &isFixed).enclosingBoundingBox();
@@ -1109,7 +1108,7 @@
return;
int delta = newText.length() - text().length();
- unsigned end = length ? offset + length - 1 : offset;
+ unsigned end = offset + length;
m_linesDirty = simpleLineLayout() || m_lineBoxes.dirtyRange(*this, offset, end, delta);
diff --git a/Source/WebCore/rendering/RenderTextLineBoxes.cpp b/Source/WebCore/rendering/RenderTextLineBoxes.cpp
index 40cac1c..6633713 100644
--- a/Source/WebCore/rendering/RenderTextLineBoxes.cpp
+++ b/Source/WebCore/rendering/RenderTextLineBoxes.cpp
@@ -515,7 +515,7 @@
static FloatRect localQuadForTextBox(const InlineTextBox& box, unsigned start, unsigned end, bool useSelectionHeight)
{
- unsigned realEnd = std::min(box.end() + 1, end);
+ unsigned realEnd = std::min(box.end(), end);
LayoutRect boxSelectionRect = box.localSelectionRect(start, realEnd);
if (!boxSelectionRect.height())
return FloatRect();
@@ -537,8 +537,7 @@
{
Vector<IntRect> rects;
for (auto* box = m_first; box; box = box->nextTextBox()) {
- // Note: box->end() returns the index of the last character, not the index past it
- if (start <= box->start() && box->end() < end) {
+ if (start <= box->start() && box->end() <= end) {
FloatRect boundaries = box->calculateBoundaries();
if (useSelectionHeight) {
LayoutRect selectionRect = box->localSelectionRect(start, end);
@@ -584,8 +583,7 @@
{
Vector<FloatQuad> quads;
for (auto* box = m_first; box; box = box->nextTextBox()) {
- // Note: box->end() returns the index of the last character, not the index past it
- if (start <= box->start() && box->end() < end) {
+ if (start <= box->start() && box->end() <= end) {
FloatRect boundaries = box->calculateBoundaries();
if (useSelectionHeight) {
LayoutRect selectionRect = box->localSelectionRect(start, end);
@@ -623,10 +621,10 @@
for (auto* current = m_first; current; current = current->nextTextBox()) {
// FIXME: This shouldn't rely on the end of a dirty line box. See https://bugs.webkit.org/show_bug.cgi?id=97264
// Text run is entirely before the affected range.
- if (current->end() < start)
+ if (current->end() <= start)
continue;
// Text run is entirely after the affected range.
- if (current->start() > end) {
+ if (current->start() >= end) {
current->offsetRun(lengthDelta);
auto& rootBox = current->root();
if (!firstRootBox) {
@@ -640,7 +638,7 @@
lastRootBox = &rootBox;
continue;
}
- if (current->end() >= start && current->end() <= end) {
+ if (current->end() > start && current->end() <= end) {
// Text run overlaps with the left end of the affected range.
current->dirtyLineBoxes();
dirtiedLines = true;
@@ -652,7 +650,7 @@
dirtiedLines = true;
continue;
}
- if (current->start() <= end && current->end() >= end) {
+ if (current->start() < end && current->end() >= end) {
// Text run overlaps with right end of the affected range.
current->dirtyLineBoxes();
dirtiedLines = true;
