InlineTextBox::isSelected() should only return true for a non-empty selection
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 11 Oct 2017 19:13:07 +0000 (19:13 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 11 Oct 2017 19:13:07 +0000 (19:13 +0000)
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:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@223196 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/rendering/InlineTextBox.cpp
Source/WebCore/rendering/InlineTextBox.h

index 2950194..a32274c 100644 (file)
@@ -1,3 +1,27 @@
+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>
 
         Sanitize URL in pasteboard for other applications and cross origin content
index d800b0a..da843a8 100644 (file)
@@ -129,15 +129,9 @@ LayoutUnit InlineTextBox::selectionHeight() const
     return root().selectionHeight();
 }
 
-bool InlineTextBox::isSelected(unsigned startPos, unsigned endPos) const
+bool InlineTextBox::isSelected(unsigned startPosition, unsigned endPosition) 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()));
+    return clampedOffset(startPosition) < clampedOffset(endPosition);
 }
 
 RenderObject::SelectionState InlineTextBox::selectionState()
@@ -197,12 +191,8 @@ LayoutRect InlineTextBox::localSelectionRect(unsigned startPos, unsigned endPos)
     unsigned sPos = clampedOffset(startPos);
     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();
     LayoutUnit selectionHeight = this->selectionHeight();
index 99f64d5..e7eb867 100644 (file)
@@ -111,7 +111,7 @@ public:
     FloatRect calculateBoundaries() const override { return FloatRect(x(), y(), width(), height()); }
 
     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;
 
 protected: