2011-01-24 Kent Tamura <tkent@chromium.org>
authortkent@chromium.org <tkent@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Jan 2011 04:36:26 +0000 (04:36 +0000)
committertkent@chromium.org <tkent@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Jan 2011 04:36:26 +0000 (04:36 +0000)
        Reviewed by Dimitri Glazkov.

        Some bugs of search cancel button and spin button about state change in
        an event handler.
        https://bugs.webkit.org/show_bug.cgi?id=46950

        * fast/forms/input-number-change-type-on-focus-expected.txt: Added.
        * fast/forms/input-number-change-type-on-focus.html: Added.
        * fast/forms/search-hide-cancel-on-cancel-expected.txt: Added.
        * fast/forms/search-hide-cancel-on-cancel.html: Added.
2011-01-24  Kent Tamura  <tkent@chromium.org>

        Reviewed by Dimitri Glazkov.

        Some bugs of search cancel button and spin button about state change in
        an event handler.
        https://bugs.webkit.org/show_bug.cgi?id=46950

        Fix the following problems:
         * Type=search field didn't release event capturing
         * Assertion failure when an input field with spin buttons was changed
           to another type on focus event.
         * A input field with spin button didn't release event capturing when it
           was changed to another type on focus event.

        Tests: fast/forms/input-number-change-type-on-focus.html
               fast/forms/search-hide-cancel-on-cancel.html

        * rendering/TextControlInnerElements.cpp:
        (WebCore::SearchFieldCancelButtonElement::defaultEventHandler):
         - Make the variable 'input' RefPtr.  It makes the code simpler.
         - Remove visibility check on mouseup event. We should release capturing
           anyway because the cancel button may be invisible if JavaScript code
           called by the focus event removes the input value.
        (WebCore::SpinButtonElement::detach):
         - Release capturing on detach because it is possible that a spin button
           node is detached while it is capturing events.
        (WebCore::SpinButtonElement::defaultEventHandler):
          Take a reference to this and check renderer() after some functions which
          may run JavaScript code.
        (WebCore::InputFieldSpeechButtonElement::defaultEventHandler):
          Make the variable 'input' RefPtr to align other functions in this file.
        (WebCore::InputFieldSpeechButtonElement::setRecognitionResult): ditto.
        * rendering/TextControlInnerElements.h: Declare SpinButtonElement::detach().

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

LayoutTests/ChangeLog
LayoutTests/fast/forms/input-number-change-type-on-focus-expected.txt [new file with mode: 0644]
LayoutTests/fast/forms/input-number-change-type-on-focus.html [new file with mode: 0644]
LayoutTests/fast/forms/search-hide-cancel-on-cancel-expected.txt [new file with mode: 0644]
LayoutTests/fast/forms/search-hide-cancel-on-cancel.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/TextControlInnerElements.cpp
Source/WebCore/rendering/TextControlInnerElements.h

index 6095d29..37b42de 100644 (file)
@@ -1,3 +1,16 @@
+2011-01-24  Kent Tamura  <tkent@chromium.org>
+
+        Reviewed by Dimitri Glazkov.
+
+        Some bugs of search cancel button and spin button about state change in
+        an event handler.
+        https://bugs.webkit.org/show_bug.cgi?id=46950
+
+        * fast/forms/input-number-change-type-on-focus-expected.txt: Added.
+        * fast/forms/input-number-change-type-on-focus.html: Added.
+        * fast/forms/search-hide-cancel-on-cancel-expected.txt: Added.
+        * fast/forms/search-hide-cancel-on-cancel.html: Added.
+
 2011-01-24  Ryosuke Niwa  <rniwa@webkit.org>
 
         Reviewed by Ojan Vafai.
diff --git a/LayoutTests/fast/forms/input-number-change-type-on-focus-expected.txt b/LayoutTests/fast/forms/input-number-change-type-on-focus-expected.txt
new file mode 100644 (file)
index 0000000..0b74457
--- /dev/null
@@ -0,0 +1,12 @@
+Bug 46950: Assertion failure by changing type from type=number in focus event.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+
+PASS Not crashed.
+PASS mouseDownCount is 1
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/forms/input-number-change-type-on-focus.html b/LayoutTests/fast/forms/input-number-change-type-on-focus.html
new file mode 100644 (file)
index 0000000..f515739
--- /dev/null
@@ -0,0 +1,52 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="../../fast/js/resources/js-test-style.css">
+<script src="../../fast/js/resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+
+<input id=number type=number value=0>
+
+<div id="console"></div>
+<script>
+description('Bug 46950: Assertion failure by changing type from type=number in focus event.');
+
+var input = document.getElementById('number');
+var spinX = input.offsetLeft + input.offsetWidth - 6;
+var middleX = input.offsetLeft + input.offsetWidth / 2
+var middleY = input.offsetTop + input.offsetHeight / 4;
+
+function changeType(event) {
+    this.type = 'text';
+}
+input.addEventListener('focus', changeType, false);
+if (window.eventSender) {
+    // Click the spin button.
+    eventSender.mouseMoveTo(spinX, middleY);
+    eventSender.mouseDown(); // This made an assertion fail.
+    eventSender.mouseUp();
+} else {
+    debug('Manual testing: Click the spin button, and see if the browser crashes or not.');
+}
+testPassed('Not crashed.');
+
+// Click the input element. The event should not be captured by the spin button.
+if (window.eventSender) {
+    var mouseDownCount = 0;
+    input.addEventListener('mousedown', function(event) {
+        mouseDownCount++;
+    }, false);
+    eventSender.mouseMoveTo(middleX, middleY);
+    eventSender.mouseDown();
+    eventSender.mouseUp();
+    shouldBe('mouseDownCount', '1');
+}
+
+
+var successfullyParsed = true;
+</script>
+<script src="../../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/fast/forms/search-hide-cancel-on-cancel-expected.txt b/LayoutTests/fast/forms/search-hide-cancel-on-cancel-expected.txt
new file mode 100644 (file)
index 0000000..48de951
--- /dev/null
@@ -0,0 +1,11 @@
+Bug 46950: Search field cancel button keeps event capturing if the value is cleared in a focus event.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+
+PASS mouseDownCount is 2
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/forms/search-hide-cancel-on-cancel.html b/LayoutTests/fast/forms/search-hide-cancel-on-cancel.html
new file mode 100644 (file)
index 0000000..2b7cbb9
--- /dev/null
@@ -0,0 +1,45 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="../../fast/js/resources/js-test-style.css">
+<script src="../../fast/js/resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+
+<input id=search type=search value=query>
+
+<div id="console"></div>
+<script>
+description('Bug 46950: Search field cancel button keeps event capturing if the value is cleared in a focus event.');
+
+var input = document.getElementById('search');
+var cancelX = input.offsetLeft + input.offsetWidth - 6;
+var middleX = input.offsetLeft + input.offsetWidth / 2
+var middleY = input.offsetTop + input.offsetHeight / 2;
+var mouseDownCount = 0;
+input.addEventListener('mousedown', function(event) {
+    mouseDownCount++;
+}, false);
+
+function clearValue(event) {
+    this.value = '';
+}
+input.addEventListener('focus', clearValue, false);
+// Click the cancel button.
+eventSender.mouseMoveTo(cancelX, middleY);
+eventSender.mouseDown();
+eventSender.mouseUp();
+// Click the input element. The event should not be captured by the cancel button.
+eventSender.mouseMoveTo(middleX, middleY);
+eventSender.mouseDown();
+eventSender.mouseUp();
+
+shouldBe('mouseDownCount', '2');
+input.removeEventListener('focus', clearValue, false);
+
+var successfullyParsed = true;
+</script>
+<script src="../../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
index 5f3fb6c..577e68d 100644 (file)
@@ -1,3 +1,38 @@
+2011-01-24  Kent Tamura  <tkent@chromium.org>
+
+        Reviewed by Dimitri Glazkov.
+
+        Some bugs of search cancel button and spin button about state change in
+        an event handler.
+        https://bugs.webkit.org/show_bug.cgi?id=46950
+
+        Fix the following problems:
+         * Type=search field didn't release event capturing
+         * Assertion failure when an input field with spin buttons was changed
+           to another type on focus event.
+         * A input field with spin button didn't release event capturing when it
+           was changed to another type on focus event.
+
+        Tests: fast/forms/input-number-change-type-on-focus.html
+               fast/forms/search-hide-cancel-on-cancel.html
+
+        * rendering/TextControlInnerElements.cpp:
+        (WebCore::SearchFieldCancelButtonElement::defaultEventHandler):
+         - Make the variable 'input' RefPtr.  It makes the code simpler.
+         - Remove visibility check on mouseup event. We should release capturing
+           anyway because the cancel button may be invisible if JavaScript code
+           called by the focus event removes the input value.
+        (WebCore::SpinButtonElement::detach):
+         - Release capturing on detach because it is possible that a spin button
+           node is detached while it is capturing events.
+        (WebCore::SpinButtonElement::defaultEventHandler):
+          Take a reference to this and check renderer() after some functions which
+          may run JavaScript code.
+        (WebCore::InputFieldSpeechButtonElement::defaultEventHandler):
+          Make the variable 'input' RefPtr to align other functions in this file.
+        (WebCore::InputFieldSpeechButtonElement::setRecognitionResult): ditto.
+        * rendering/TextControlInnerElements.h: Declare SpinButtonElement::detach().
+
 2011-01-24  Ryosuke Niwa  <rniwa@webkit.org>
 
         Reviewed by Ojan Vafai.
index 9825ad1..1162999 100644 (file)
@@ -219,7 +219,7 @@ void SearchFieldCancelButtonElement::detach()
 void SearchFieldCancelButtonElement::defaultEventHandler(Event* event)
 {
     // If the element is visible, on mouseup, clear the value, and set selection
-    HTMLInputElement* input = static_cast<HTMLInputElement*>(shadowAncestorNode());
+    RefPtr<HTMLInputElement> input(static_cast<HTMLInputElement*>(shadowAncestorNode()));
     if (event->type() == eventNames().mousedownEvent && event->isMouseEvent() && static_cast<MouseEvent*>(event)->button() == LeftButton) {
         if (renderer() && renderer()->visibleToHitTesting()) {
             if (Frame* frame = document()->frame()) {
@@ -232,13 +232,12 @@ void SearchFieldCancelButtonElement::defaultEventHandler(Event* event)
         event->setDefaultHandled();
     }
     if (event->type() == eventNames().mouseupEvent && event->isMouseEvent() && static_cast<MouseEvent*>(event)->button() == LeftButton) {
-        if (m_capturing && renderer() && renderer()->visibleToHitTesting()) {
+        if (m_capturing) {
             if (Frame* frame = document()->frame()) {
                 frame->eventHandler()->setCapturingMouseEventsNode(0);
                 m_capturing = false;
             }
             if (hovered()) {
-                RefPtr<HTMLInputElement> protector(input);
                 String oldValue = input->value();
                 input->setValue("");
                 if (!oldValue.isEmpty()) {
@@ -271,6 +270,18 @@ PassRefPtr<SpinButtonElement> SpinButtonElement::create(HTMLElement* shadowParen
     return adoptRef(new SpinButtonElement(shadowParent));
 }
 
+void SpinButtonElement::detach()
+{
+    stopRepeatingTimer();
+    if (m_capturing) {
+        if (Frame* frame = document()->frame()) {
+            frame->eventHandler()->setCapturingMouseEventsNode(0);
+            m_capturing = false;
+        }
+    }
+    TextControlInnerElement::detach();
+}
+
 void SpinButtonElement::defaultEventHandler(Event* event)
 {
     if (!event->isMouseEvent()) {
@@ -286,7 +297,7 @@ void SpinButtonElement::defaultEventHandler(Event* event)
         return;
     }
 
-    HTMLInputElement* input = static_cast<HTMLInputElement*>(shadowAncestorNode());
+    RefPtr<HTMLInputElement> input(static_cast<HTMLInputElement*>(shadowAncestorNode()));
     if (input->disabled() || input->isReadOnlyFormControl()) {
         if (!event->defaultHandled())
             HTMLDivElement::defaultEventHandler(event);
@@ -297,12 +308,18 @@ void SpinButtonElement::defaultEventHandler(Event* event)
     IntPoint local = roundedIntPoint(box->absoluteToLocal(mouseEvent->absoluteLocation(), false, true));
     if (mouseEvent->type() == eventNames().mousedownEvent && mouseEvent->button() == LeftButton) {
         if (box->borderBoxRect().contains(local)) {
-            RefPtr<Node> protector(input);
+            // The following functions of HTMLInputElement may run JavaScript
+            // code which detaches this shadow node. We need to take a reference
+            // and check renderer() after such function calls.
+            RefPtr<Node> protector(this);
             input->focus();
             input->select();
-            input->stepUpFromRenderer(m_upDownState == Up ? 1 : -1);
+            if (renderer()) {
+                input->stepUpFromRenderer(m_upDownState == Up ? 1 : -1);
+                if (renderer())
+                    startRepeatingTimer();
+            }
             event->setDefaultHandled();
-            startRepeatingTimer();
         }
     } else if (mouseEvent->type() == eventNames().mouseupEvent && mouseEvent->button() == LeftButton)
         stopRepeatingTimer();
@@ -403,8 +420,12 @@ void InputFieldSpeechButtonElement::defaultEventHandler(Event* event)
         return;
     }
 
+    // The call to focus() below dispatches a focus event, and an event handler in the page might
+    // remove the input element from DOM. To make sure it remains valid until we finish our work
+    // here, we take a temporary reference.
+    RefPtr<HTMLInputElement> input(static_cast<HTMLInputElement*>(shadowAncestorNode()));
+
     // On mouse down, select the text and set focus.
-    HTMLInputElement* input = static_cast<HTMLInputElement*>(shadowAncestorNode());
     if (event->type() == eventNames().mousedownEvent && event->isMouseEvent() && static_cast<MouseEvent*>(event)->button() == LeftButton) {
         if (renderer() && renderer()->visibleToHitTesting()) {
             if (Frame* frame = document()->frame()) {
@@ -412,10 +433,6 @@ void InputFieldSpeechButtonElement::defaultEventHandler(Event* event)
                 m_capturing = true;
             }
         }
-        // The call to focus() below dispatches a focus event, and an event handler in the page might
-        // remove the input element from DOM. To make sure it remains valid until we finish our work
-        // here, we take a temporary reference.
-        RefPtr<HTMLInputElement> holdRef(input);
         RefPtr<InputFieldSpeechButtonElement> holdRefButton(this);
         input->focus();
         input->select();
@@ -482,11 +499,10 @@ void InputFieldSpeechButtonElement::setRecognitionResult(int, const SpeechInputR
 {
     m_results = results;
 
-    HTMLInputElement* input = static_cast<HTMLInputElement*>(shadowAncestorNode());
     // The call to setValue() below dispatches an event, and an event handler in the page might
     // remove the input element from DOM. To make sure it remains valid until we finish our work
     // here, we take a temporary reference.
-    RefPtr<HTMLInputElement> holdRef(input);
+    RefPtr<HTMLInputElement> input(static_cast<HTMLInputElement*>(shadowAncestorNode()));
     RefPtr<InputFieldSpeechButtonElement> holdRefButton(this);
     input->setValue(results.isEmpty() ? "" : results[0]->utterance());
     input->dispatchEvent(SpeechInputEvent::create(eventNames().webkitspeechchangeEvent, results));
index 0cd0596..4ba7857 100644 (file)
@@ -99,6 +99,7 @@ public:
 private:
     SpinButtonElement(HTMLElement*);
 
+    virtual void detach();
     virtual bool isSpinButtonElement() const { return true; }
     // FIXME: shadowAncestorNode() should be const.
     virtual bool isEnabledFormControl() const { return static_cast<Element*>(const_cast<SpinButtonElement*>(this)->shadowAncestorNode())->isEnabledFormControl(); }