Attachment 69417 Details for Bug 46950 – Patch

[patch] Patch

bug-46950-20101001121801.patch (text/plain), 10.12 KB, created by Kent Tamura on 2010-09-30 20:18:03 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Kent Tamura

Created: 2010-09-30 20:18:03 PDT

Size: 10.12 KB

patch

obsolete

>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 4167bb87a450af3ef19e5d594c57758d570acacb..de69772d3b18d4c53d3780234912db1c2064900a 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,16 @@
>+2010-09-30  Kent Tamura  <tkent@chromium.org>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        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.
>+
> 2010-09-29  Eric Seidel  <eric@webkit.org>
> 
>         Unreviewed.  Skipping crashing test to green the tree.
>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
>index 0000000000000000000000000000000000000000..5cd40c25e6db9f2982b7c158b90cf6d104578abc
>--- /dev/null
>+++ b/LayoutTests/fast/forms/input-number-change-type-on-focus-expected.txt
>@@ -0,0 +1,11 @@
>+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 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
>index 0000000000000000000000000000000000000000..61031a7c637e7daa4dc5873a9dfe70b1a1872150
>--- /dev/null
>+++ b/LayoutTests/fast/forms/input-number-change-type-on-focus.html
>@@ -0,0 +1,38 @@
>+<!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 cancelX = input.offsetLeft + input.offsetWidth - 6;
>+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(cancelX, 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.');
>+
>+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
>index 0000000000000000000000000000000000000000..48de951c9ad38bc7c4e5b66a6667b347c3fefd0d
>--- /dev/null
>+++ b/LayoutTests/fast/forms/search-hide-cancel-on-cancel-expected.txt
>@@ -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
>index 0000000000000000000000000000000000000000..2b7cbb9b6663974d1b1d24420de5c1f362815055
>--- /dev/null
>+++ b/LayoutTests/fast/forms/search-hide-cancel-on-cancel.html
>@@ -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>
>diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
>index b7a599de71479a66304a9e71cef6dd91e4b7df63..8c19c1b5cd0e092e4ae7b43f46e5917f7e514d67 100644
>--- a/WebCore/ChangeLog
>+++ b/WebCore/ChangeLog
>@@ -1,3 +1,24 @@
>+2010-09-30  Kent Tamura  <tkent@chromium.org>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        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
>+
>+        Tests: fast/forms/input-number-change-type-on-focus.html
>+               fast/forms/search-hide-cancel-on-cancel.html
>+
>+        * rendering/TextControlInnerElements.cpp:
>+        (WebCore::SearchFieldCancelButtonElement::defaultEventHandler):
>+         Capturing mouse events after HTMLInputElement::focus() because a
>+         focus event handler can clear the value and hides the cancel button.
>+        (WebCore::SpinButtonElement::detach):
>+         Release event capturing when the node is detached.
>+        (WebCore::SpinButtonElement::defaultEventHandler):
>+         Check if the node is attached before calling stepUpFromRenderer().
>+        * rendering/TextControlInnerElements.h:
>+
> 2010-09-29  Mihai Parparita  <mihaip@chromium.org>
> 
>         Reviewed by Darin Fisher.
>diff --git a/WebCore/rendering/TextControlInnerElements.cpp b/WebCore/rendering/TextControlInnerElements.cpp
>index e5228f0771fdc3e0049684da2018ca64832af49d..23724aab3304ff3d72ee0b85b33ed9d097817864 100644
>--- a/WebCore/rendering/TextControlInnerElements.cpp
>+++ b/WebCore/rendering/TextControlInnerElements.cpp
>@@ -219,14 +219,18 @@ void SearchFieldCancelButtonElement::defaultEventHandler(Event* event)
>     // If the element is visible, on mouseup, clear the value, and set selection
>     HTMLInputElement* input = static_cast<HTMLInputElement*>(shadowAncestorNode());
>     if (event->type() == eventNames().mousedownEvent && event->isMouseEvent() && static_cast<MouseEvent*>(event)->button() == LeftButton) {
>+        RefPtr<HTMLInputElement> protector(input);
>+        RefPtr<Node> protector2(this);
>+        input->focus();
>+        input->select();
>+        // By an event handler, the input element might be removed or hidden,
>+        // the cancel button might be hidden, or the input type might be changed.
>         if (renderer() && renderer()->visibleToHitTesting()) {
>             if (Frame* frame = document()->frame()) {
>                 frame->eventHandler()->setCapturingMouseEventsNode(this);
>                 m_capturing = true;
>             }
>         }
>-        input->focus();
>-        input->select();
>         event->setDefaultHandled();
>     }
>     if (event->type() == eventNames().mouseupEvent && event->isMouseEvent() && static_cast<MouseEvent*>(event)->button() == LeftButton) {
>@@ -269,6 +273,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()) {
>@@ -296,11 +312,16 @@ void SpinButtonElement::defaultEventHandler(Event* event)
>     if (mouseEvent->type() == eventNames().mousedownEvent && mouseEvent->button() == LeftButton) {
>         if (box->borderBoxRect().contains(local)) {
>             RefPtr<Node> protector(input);
>+            RefPtr<Node> protector2(this);
>             input->focus();
>             input->select();
>-            input->stepUpFromRenderer(m_upDownState == Up ? 1 : -1);
>+            // Input type might be changed by an event handler. This shadow node might be detached.
>+            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();
>diff --git a/WebCore/rendering/TextControlInnerElements.h b/WebCore/rendering/TextControlInnerElements.h
>index c146a42dc4349e77ddabee317da6e71ef56a7e58..36907608c67c4c1a9fb4ec0bb2ecf4eef0cb47bc 100644
>--- a/WebCore/rendering/TextControlInnerElements.h
>+++ b/WebCore/rendering/TextControlInnerElements.h
>@@ -103,6 +103,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(); }

Actions: View | Formatted Diff | Diff

Attachments on bug 46950: 69417 | 73039 | 73041 | 76495 | 79903