Changeset 223196 in webkit

Timestamp:

Oct 11, 2017 12:13:07 PM (7 years ago)

Author:

dbates@webkit.org

Message:

InlineTextBox::isSelected() should only return true for a non-empty selection
and remove incorrect FIXME from InlineTextBox::localSelectionRect()
​https://bugs.webkit.org/show_bug.cgi?id=160786

Reviewed by Zalan Bujtas.

Partial revert of r204400 in InlineTextBox::{isSelected, localSelectionRect}().

The function InlineTextBox::isSelected() should only return true for a non-empty selection.
Also remove an incorrect FIXME added to InlineTextBox::localSelectionRect() that questioned
whether it was correct for it to return an empty rectangle. It is correct for it to return
such a rectangle because this function is used to implement Element.getClientRects(). And
Element.getClientRects() can return a rectangle with zero width or zero height by step 3
of algorithm getClientRects() of section Extensions to the Element interface of the
CSSOM View Module spec., <​https://drafts.csswg.org/cssom-view/> (Editor's Draft, 15 September 2017).

• rendering/InlineTextBox.cpp:

(WebCore::InlineTextBox::isSelected const): Only return true for a non-empty selection
and remove unnecessary FIXME. Also rename variables to improve readability.
(WebCore::InlineTextBox::localSelectionRect const): Remove inaccurate FIXME comment.

• rendering/InlineTextBox.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• rendering/InlineTextBox.cpp (modified) (2 diffs)
• rendering/InlineTextBox.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-10-11  Daniel Bates  <dabates@apple.com>
+
+        InlineTextBox::isSelected() should only return true for a non-empty selection
+        and remove incorrect FIXME from InlineTextBox::localSelectionRect()
+        https://bugs.webkit.org/show_bug.cgi?id=160786
+
+        Reviewed by Zalan Bujtas.
+
+        Partial revert of r204400 in InlineTextBox::{isSelected, localSelectionRect}().
+
+        The function InlineTextBox::isSelected() should only return true for a non-empty selection.
+        Also remove an incorrect FIXME added to InlineTextBox::localSelectionRect() that questioned
+        whether it was correct for it to return an empty rectangle. It is correct for it to return
+        such a rectangle because this function is used to implement Element.getClientRects(). And
+        Element.getClientRects() can return a rectangle with zero width or zero height by step 3
+        of algorithm getClientRects() of section Extensions to the Element interface of the
+        CSSOM View Module spec., <https://drafts.csswg.org/cssom-view/> (Editor's Draft, 15 September 2017).
+
+        * rendering/InlineTextBox.cpp:
+        (WebCore::InlineTextBox::isSelected const): Only return true for a non-empty selection
+        and remove unnecessary FIXME. Also rename variables to improve readability.
+        (WebCore::InlineTextBox::localSelectionRect const): Remove inaccurate FIXME comment.
+        * rendering/InlineTextBox.h:
+
 2017-10-11  Ryosuke Niwa  <rniwa@webkit.org>
 

trunk/Source/WebCore/rendering/InlineTextBox.cpp

@@ -130 +130 @@
 }
 
-bool InlineTextBox::isSelected(unsigned startPos, unsigned endPos) const
-{
-    int sPos = clampedOffset(startPos);
-    int ePos = clampedOffset(endPos);
-    // FIXME: https://bugs.webkit.org/show_bug.cgi?id=160786
-    // We should only be checking if sPos >= ePos here, because those are the
-    // indices used to actually generate the selection rect. Allowing us past this guard
-    // on any other condition creates zero-width selection rects.
-    return sPos < ePos || (startPos == endPos && startPos >= start() && startPos <= (start() + len()));
+bool InlineTextBox::isSelected(unsigned startPosition, unsigned endPosition) const
+{
+    return clampedOffset(startPosition) < clampedOffset(endPosition);
 }
 
@@ -198 +192 @@
     unsigned ePos = clampedOffset(endPos);
 
-    // FIXME: https://bugs.webkit.org/show_bug.cgi?id=160786
-    // We should only be checking if sPos >= ePos here, because those are the
-    // indices used to actually generate the selection rect. Allowing us past this guard
-    // on any other condition creates zero-width selection rects.
     if (sPos >= ePos && !(startPos == endPos && startPos >= start() && startPos <= (start() + len())))
-        return LayoutRect();
+        return { };
 
     LayoutUnit selectionTop = this->selectionTop();

trunk/Source/WebCore/rendering/InlineTextBox.h

@@ -112 +112 @@
 
     virtual LayoutRect localSelectionRect(unsigned startPos, unsigned endPos) const;
-    bool isSelected(unsigned startPos, unsigned endPos) const;
+    bool isSelected(unsigned startPosition, unsigned endPosition) const;
     std::pair<unsigned, unsigned> selectionStartEnd() const;