Browse files

Text selection in text area in auto scroll mode goes wrong.

https://bugs.webkit.org/show_bug.cgi?id=74346

Patch by Sukolsak Sakshuwong <sukolsak@google.com> on 2012-08-14
Reviewed by Ojan Vafai.

Source/WebCore:

WebKit triggers autoscroll in text area when the user drags the cursor from inside
the text area to the outside. When that happens, it gets the local cursor position
relative to the node under the cursor from hit-testing, converts it to
the absolute position, and then converts it to the local position relative to the
text area. However, the hit-testing method of text area did not take scrolling
offset into account. This caused it to give an incorrect value of the local cursor
position. Make the hit-testing take scrolling offset into account.

Test: fast/events/autoscroll-in-textarea.html

* html/shadow/TextControlInnerElements.cpp:
(WebCore::TextControlInnerTextElement::createRenderer):
* rendering/RenderTextControl.cpp:
(WebCore::RenderTextControl::hitInnerTextElement):
* rendering/RenderTextControlSingleLine.cpp:
(WebCore):
* rendering/RenderTextControlSingleLine.h:
(WebCore::RenderTextControlInnerBlock::RenderTextControlInnerBlock):
(WebCore::RenderTextControlInnerBlock::hasLineIfEmpty):

LayoutTests:

* fast/events/autoscroll-in-textarea-expected.txt: Added.
* fast/events/autoscroll-in-textarea.html: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@125648 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information...
1 parent e4000ce commit 04f91535881c69d5fd342d6fffdcbc22f5ff3aeb commit-queue committed Aug 15, 2012
View
10 LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2012-08-14 Sukolsak Sakshuwong <sukolsak@google.com>
+
+ Text selection in text area in auto scroll mode goes wrong.
+ https://bugs.webkit.org/show_bug.cgi?id=74346
+
+ Reviewed by Ojan Vafai.
+
+ * fast/events/autoscroll-in-textarea-expected.txt: Added.
+ * fast/events/autoscroll-in-textarea.html: Added.
+
2012-08-14 Yuta Kitamura <yutak@google.com>
[Chromium] Unreviewed. Remove obsolete test expectations, and mark
View
7 LayoutTests/fast/events/autoscroll-in-textarea-expected.txt
@@ -0,0 +1,7 @@
+
+
+
+
+This tests autoscroll in text area correctly shows selection highlight. To manually test, scroll the text area down to the end. Slowly drag up your mouse starting from the line closest to the upper edge of the text area. Once the contents of the text area get scrolled a little, the selection should not jump down to the end.
+
+PASSED the selection did not jump down.
View
67 LayoutTests/fast/events/autoscroll-in-textarea.html
@@ -0,0 +1,67 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+#textarea {
+ font-size: 18px;
+ width: 400px;
+ margin: 0;
+ padding: 0;
+}
+</style>
+</head>
+<body>
+<br><br><br>
+<textarea id="textarea" rows="6">
+a
+b
+c
+d
+e
+f
+g
+h
+i
+j
+k
+l
+m
+n
+o
+p</textarea>
+
+<p>This tests autoscroll in text area correctly shows selection highlight.
+To manually test, scroll the text area down to the end.
+Slowly drag up your mouse starting from the line closest to the upper edge of the text area.
+Once the contents of the text area get scrolled a little,
+the selection should not jump down to the end.</p>
+<div id="log"></div>
+
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+
+// The text area displays 6 lines of text. We scroll down to the end
+// and start dragging the cursor up from the first line that appears (the k line).
+// Therefore, the selection should contain the letter k.
+var textarea = document.getElementById("textarea");
+textarea.scrollTop = textarea.scrollHeight;
+if (window.eventSender) {
+ var x = textarea.offsetLeft + textarea.offsetWidth / 2;
+ var y = textarea.offsetTop + 1;
+ eventSender.dragMode = false;
+ eventSender.mouseMoveTo(x, y);
+ eventSender.mouseDown();
+ eventSender.mouseMoveTo(x, 0);
+ eventSender.mouseUp();
+
+ var log = document.getElementById("log");
+ var selectedText = window.getSelection().toString();
+ if (selectedText.indexOf("k") != -1)
+ log.innerText = "PASSED the selection did not jump down.";
+ else
+ log.innerText = "FAILED the selection jumped down.";
+}
+</script>
+</body>
+</html>
View
27 Source/WebCore/ChangeLog
@@ -1,3 +1,30 @@
+2012-08-14 Sukolsak Sakshuwong <sukolsak@google.com>
+
+ Text selection in text area in auto scroll mode goes wrong.
+ https://bugs.webkit.org/show_bug.cgi?id=74346
+
+ Reviewed by Ojan Vafai.
+
+ WebKit triggers autoscroll in text area when the user drags the cursor from inside
+ the text area to the outside. When that happens, it gets the local cursor position
+ relative to the node under the cursor from hit-testing, converts it to
+ the absolute position, and then converts it to the local position relative to the
+ text area. However, the hit-testing method of text area did not take scrolling
+ offset into account. This caused it to give an incorrect value of the local cursor
+ position. Make the hit-testing take scrolling offset into account.
+
+ Test: fast/events/autoscroll-in-textarea.html
+
+ * html/shadow/TextControlInnerElements.cpp:
+ (WebCore::TextControlInnerTextElement::createRenderer):
+ * rendering/RenderTextControl.cpp:
+ (WebCore::RenderTextControl::hitInnerTextElement):
+ * rendering/RenderTextControlSingleLine.cpp:
+ (WebCore):
+ * rendering/RenderTextControlSingleLine.h:
+ (WebCore::RenderTextControlInnerBlock::RenderTextControlInnerBlock):
+ (WebCore::RenderTextControlInnerBlock::hasLineIfEmpty):
+
2012-08-14 Shinya Kawanaka <shinyak@chromium.org>
[Refactoring] RenderMenuList and RenderListBox should have a method to return HTMLSelectElement.
View
8 Source/WebCore/html/shadow/TextControlInnerElements.cpp
@@ -102,13 +102,7 @@ void TextControlInnerTextElement::defaultEventHandler(Event* event)
RenderObject* TextControlInnerTextElement::createRenderer(RenderArena* arena, RenderStyle*)
{
- bool multiLine = false;
- Element* shadowAncestor = shadowHost();
- if (shadowAncestor && shadowAncestor->renderer()) {
- ASSERT(shadowAncestor->renderer()->isTextField() || shadowAncestor->renderer()->isTextArea());
- multiLine = shadowAncestor->renderer()->isTextArea();
- }
- return new (arena) RenderTextControlInnerBlock(this, multiLine);
+ return new (arena) RenderTextControlInnerBlock(this);
}
PassRefPtr<RenderStyle> TextControlInnerTextElement::customStyleForRenderer()
View
7 Source/WebCore/rendering/RenderTextControl.cpp
@@ -155,11 +155,14 @@ void RenderTextControl::computeLogicalHeight()
void RenderTextControl::hitInnerTextElement(HitTestResult& result, const LayoutPoint& pointInContainer, const LayoutPoint& accumulatedOffset)
{
- LayoutPoint adjustedLocation = accumulatedOffset + location();
HTMLElement* innerText = innerTextElement();
+ LayoutPoint adjustedLocation = accumulatedOffset + location();
+ LayoutPoint localPoint = pointInContainer - toLayoutSize(adjustedLocation + innerText->renderBox()->location());
+ if (hasOverflowClip())
+ localPoint += scrolledContentOffset();
result.setInnerNode(innerText);
result.setInnerNonSharedNode(innerText);
- result.setLocalPoint(pointInContainer - toLayoutSize(adjustedLocation + innerText->renderBox()->location()));
+ result.setLocalPoint(localPoint);
}
static const char* fontFamiliesWithInvalidCharWidth[] = {
View
17 Source/WebCore/rendering/RenderTextControlSingleLine.cpp
@@ -49,23 +49,6 @@ namespace WebCore {
using namespace HTMLNames;
-VisiblePosition RenderTextControlInnerBlock::positionForPoint(const LayoutPoint& point)
-{
- LayoutPoint contentsPoint(point);
-
- // Multiline text controls have the scroll on shadowHost, so we need to take
- // that into account here.
- if (m_multiLine) {
- RenderTextControl* renderer = toRenderTextControl(node()->shadowHost()->renderer());
- if (renderer->hasOverflowClip())
- contentsPoint += renderer->scrolledContentOffset();
- }
-
- return RenderBlock::positionForPoint(contentsPoint);
-}
-
-// ----------------------------
-
RenderTextControlSingleLine::RenderTextControlSingleLine(Node* node)
: RenderTextControl(node)
, m_shouldDrawCapsLockIndicator(false)
View
5 Source/WebCore/rendering/RenderTextControlSingleLine.h
@@ -110,13 +110,10 @@ void toRenderTextControlSingleLine(const RenderTextControlSingleLine*);
class RenderTextControlInnerBlock : public RenderBlock {
public:
- RenderTextControlInnerBlock(Node* node, bool isMultiLine) : RenderBlock(node), m_multiLine(isMultiLine) { }
+ RenderTextControlInnerBlock(Node* node) : RenderBlock(node) { }
private:
virtual bool hasLineIfEmpty() const { return true; }
- virtual VisiblePosition positionForPoint(const LayoutPoint&);
-
- bool m_multiLine;
};
}

0 comments on commit 04f9153

Please sign in to comment.