Fix <https://bugs.webkit.org/show_bug.cgi?id=23858>
authormrowe@apple.com <mrowe@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Feb 2009 03:13:06 +0000 (03:13 +0000)
committermrowe@apple.com <mrowe@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Feb 2009 03:13:06 +0000 (03:13 +0000)
Bug 23858: Crash when removing a HTMLSelectElement from the document from inside its focus event handler

Reviewed by Darin Adler.

* html/HTMLSelectElement.cpp:
(WebCore::HTMLSelectElement::menuListDefaultEventHandler): Don't store the renderer in a local variable
as it can be invalidated by any of the calls to focus() within the function.  Instead, retrieve it and
null-check it when it is needed.

Test for <https://bugs.webkit.org/show_bug.cgi?id=23858>
Bug 23858: Crash when removing a HTMLSelectElement from the document from inside its focus event handler

Reviewed by Sam Weinig.

* fast/dom/HTMLSelectElement/remove-element-from-within-focus-handler-crash-expected.txt: Added.
* fast/dom/HTMLSelectElement/remove-element-from-within-focus-handler-crash.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/dom/HTMLSelectElement/remove-element-from-within-focus-handler-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/HTMLSelectElement/remove-element-from-within-focus-handler-crash.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/html/HTMLSelectElement.cpp

index e536611..0e4844f 100644 (file)
@@ -1,3 +1,13 @@
+2009-02-09  Mark Rowe  <mrowe@apple.com>
+
+        Reviewed by Sam Weinig.
+
+        Test for <https://bugs.webkit.org/show_bug.cgi?id=23858>
+        Bug 23858: Crash when removing a HTMLSelectElement from the document from inside its focus event handler
+
+        * fast/dom/HTMLSelectElement/remove-element-from-within-focus-handler-crash-expected.txt: Added.
+        * fast/dom/HTMLSelectElement/remove-element-from-within-focus-handler-crash.html: Added.
+
 2009-02-09  Dimitri Glazkov  <dglazkov@chromium.org>
 
         Reviewed by Dave Hyatt.
diff --git a/LayoutTests/fast/dom/HTMLSelectElement/remove-element-from-within-focus-handler-crash-expected.txt b/LayoutTests/fast/dom/HTMLSelectElement/remove-element-from-within-focus-handler-crash-expected.txt
new file mode 100644 (file)
index 0000000..b4d9c64
--- /dev/null
@@ -0,0 +1,3 @@
+Layout test for bug 23858
+
+If this page is displayed without crashing then the test has passed.
diff --git a/LayoutTests/fast/dom/HTMLSelectElement/remove-element-from-within-focus-handler-crash.html b/LayoutTests/fast/dom/HTMLSelectElement/remove-element-from-within-focus-handler-crash.html
new file mode 100644 (file)
index 0000000..4b48a41
--- /dev/null
@@ -0,0 +1,14 @@
+<select id="bomb" onfocus="this.parentNode.removeChild(this)">
+    <option>Clicking on this select element should not crash</option>
+</select>
+<h2>Layout test for <a href='https://bugs.webkit.org/show_bug.cgi?id=23858'>bug 23858</a></h2>
+<p>If this page is displayed without crashing then the test has passed.</p>
+<script>
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+
+    var select = document.getElementById('bomb');
+    var mouseEvent = document.createEvent("MouseEvents");
+    mouseEvent.initMouseEvent("mousedown", true, true, document.defaultView, 1, select.offsetLeft + 1, select.offsetTop + 1, select.offsetLeft + 1, select.offsetTop + 1, false, false, false, false, 0, document);
+    select.dispatchEvent(mouseEvent);
+</script>
index a70bfb5..20b4d49 100644 (file)
@@ -1,3 +1,15 @@
+2009-02-09  Mark Rowe  <mrowe@apple.com>
+
+        Reviewed by Darin Adler.
+
+        Fix <https://bugs.webkit.org/show_bug.cgi?id=23858>
+        Bug 23858: Crash when removing a HTMLSelectElement from the document from inside its focus event handler
+
+        * html/HTMLSelectElement.cpp:
+        (WebCore::HTMLSelectElement::menuListDefaultEventHandler): Don't store the renderer in a local variable
+        as it can be invalidated by any of the calls to focus() within the function.  Instead, retrieve it and
+        null-check it when it is needed.
+
 2009-02-09  David Hyatt  <hyatt@apple.com>
 
         Remove the m_height member from InlineBox.  This shaves 4 bytes off of all inline boxes.  Unfortunately SVG
index 8e0e2c7..fd34a4b 100644 (file)
@@ -617,8 +617,6 @@ void HTMLSelectElement::defaultEventHandler(Event* evt)
 
 void HTMLSelectElement::menuListDefaultEventHandler(Event* evt)
 {
-    RenderMenuList* menuList = static_cast<RenderMenuList*>(renderer());
-
     if (evt->type() == eventNames().keydownEvent) {
         if (!renderer() || !evt->isKeyboardEvent())
             return;
@@ -630,7 +628,8 @@ void HTMLSelectElement::menuListDefaultEventHandler(Event* evt)
             // Save the selection so it can be compared to the new selection when we call onChange during setSelectedIndex,
             // which gets called from RenderMenuList::valueChanged, which gets called after the user makes a selection from the menu.
             saveLastSelection();
-            menuList->showPopup();
+            if (RenderMenuList* menuList = static_cast<RenderMenuList*>(renderer()))
+                menuList->showPopup();
             handled = true;
         }
 #else
@@ -672,7 +671,8 @@ void HTMLSelectElement::menuListDefaultEventHandler(Event* evt)
             // Save the selection so it can be compared to the new selection when we call onChange during setSelectedIndex,
             // which gets called from RenderMenuList::valueChanged, which gets called after the user makes a selection from the menu.
             saveLastSelection();
-            menuList->showPopup();
+            if (RenderMenuList* menuList = static_cast<RenderMenuList*>(renderer()))
+                menuList->showPopup();
             handled = true;
         }
         if (keyCode == '\r') {
@@ -695,13 +695,15 @@ void HTMLSelectElement::menuListDefaultEventHandler(Event* evt)
 
     if (evt->type() == eventNames().mousedownEvent && evt->isMouseEvent() && static_cast<MouseEvent*>(evt)->button() == LeftButton) {
         focus();
-        if (menuList->popupIsVisible())
-            menuList->hidePopup();
-        else {
-            // Save the selection so it can be compared to the new selection when we call onChange during setSelectedIndex,
-            // which gets called from RenderMenuList::valueChanged, which gets called after the user makes a selection from the menu.
-            saveLastSelection();
-            menuList->showPopup();
+        if (RenderMenuList* menuList = static_cast<RenderMenuList*>(renderer())) {
+            if (menuList->popupIsVisible())
+                menuList->hidePopup();
+            else {
+                // Save the selection so it can be compared to the new selection when we call onChange during setSelectedIndex,
+                // which gets called from RenderMenuList::valueChanged, which gets called after the user makes a selection from the menu.
+                saveLastSelection();
+                menuList->showPopup();
+            }
         }
         evt->setDefaultHandled();
     }