Changeset 76566 in webkit

Timestamp:

Jan 24, 2011 8:36:26 PM (13 years ago)

Author:

tkent@chromium.org

Message:

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 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().

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/forms/input-number-change-type-on-focus-expected.txt (added)
• LayoutTests/fast/forms/input-number-change-type-on-focus.html (added)
• LayoutTests/fast/forms/search-hide-cancel-on-cancel-expected.txt (added)
• LayoutTests/fast/forms/search-hide-cancel-on-cancel.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/TextControlInnerElements.cpp (modified) (9 diffs)
• Source/WebCore/rendering/TextControlInnerElements.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/rendering/TextControlInnerElements.cpp

@@ -220 +220 @@
 {
     // 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()) {
@@ -233 +233 @@
     }
     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);
@@ -239 +239 @@
             }
             if (hovered()) {
-                RefPtr<HTMLInputElement> protector(input);
                 String oldValue = input->value();
                 input->setValue("");
@@ -272 +271 @@
 }
 
+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)
 {
@@ -287 +298 @@
     }
 
-    HTMLInputElement* input = static_cast<HTMLInputElement*>(shadowAncestorNode());
+    RefPtr<HTMLInputElement> input(static_cast<HTMLInputElement*>(shadowAncestorNode()));
     if (input->disabled() || input->isReadOnlyFormControl()) {
         if (!event->defaultHandled())
@@ -298 +309 @@
     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)
@@ -404 +421 @@
     }
 
+    // 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()) {
@@ -413 +434 @@
             }
         }
-        // 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();
@@ -483 +500 @@
     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());

trunk/Source/WebCore/rendering/TextControlInnerElements.h

@@ -100 +100 @@
     SpinButtonElement(HTMLElement*);
 
+    virtual void detach();
     virtual bool isSpinButtonElement() const { return true; }
     // FIXME: shadowAncestorNode() should be const.