InlineTextBox::end() should return first-past-end offset
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Aug 2019 19:29:53 +0000 (19:29 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Aug 2019 19:29:53 +0000 (19:29 +0000)
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: https://svn.webkit.org/repository/webkit/trunk@249160 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/dom/Position.cpp
Source/WebCore/layout/Verification.cpp
Source/WebCore/rendering/InlineFlowBox.cpp
Source/WebCore/rendering/InlineTextBox.cpp
Source/WebCore/rendering/InlineTextBox.h
Source/WebCore/rendering/RenderText.cpp
Source/WebCore/rendering/RenderTextLineBoxes.cpp

index a3ea61c..33d3be6 100644 (file)
@@ -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
index 8b05bc8..6d584ed 100644 (file)
@@ -873,7 +873,10 @@ Position Position::downstream(EditingBoundaryCrossingRule rule) const
             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;
index ccf46d8..3323b12 100644 (file)
@@ -120,7 +120,7 @@ static bool checkForMatchingTextRuns(const Display::Run& inlineRun, const Inline
         && 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 @@ static bool outputMismatchingComplexLineInformationIfNeeded(TextStream& stream,
             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";
index 2a1bc28..96a249f 100644 (file)
@@ -392,7 +392,7 @@ float InlineFlowBox::placeBoxRangeInInlineDirection(InlineBox* firstChild, Inlin
             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())
index dff8b11..90aeacf 100644 (file)
@@ -554,7 +554,7 @@ void InlineTextBox::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset,
     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 @@ FloatRect InlineTextBox::calculateDocumentMarkerBounds(const MarkedText& markedT
     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 @@ Vector<MarkedText> InlineTextBox::collectMarkedTextsForDocumentMarkers(TextPaint
             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 @@ void InlineTextBox::paintCompositionUnderlines(PaintInfo& paintInfo, const Float
             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 @@ void InlineTextBox::paintCompositionUnderline(PaintInfo& paintInfo, const FloatP
     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;
index 52348a5..5ca2bc8 100644 (file)
@@ -66,7 +66,7 @@ public:
     // 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; }
index b6305de..6b432f3 100644 (file)
@@ -360,11 +360,10 @@ void RenderText::collectSelectionRects(Vector<SelectionRect>& rects, unsigned st
 
     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 @@ void RenderText::collectSelectionRects(Vector<SelectionRect>& rects, unsigned st
         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 @@ void RenderText::setTextWithOffset(const String& newText, unsigned offset, unsig
         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);
 
index 40cac1c..6633713 100644 (file)
@@ -515,7 +515,7 @@ Vector<IntRect> RenderTextLineBoxes::absoluteRects(const LayoutPoint& accumulate
 
 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> RenderTextLineBoxes::absoluteRectsForRange(const RenderText& ren
 {
     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> RenderTextLineBoxes::absoluteQuadsForRange(const RenderText& r
 {
     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 @@ bool RenderTextLineBoxes::dirtyRange(RenderText& renderer, unsigned start, unsig
     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 @@ bool RenderTextLineBoxes::dirtyRange(RenderText& renderer, unsigned start, unsig
             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 @@ bool RenderTextLineBoxes::dirtyRange(RenderText& renderer, unsigned start, unsig
             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;