[Qt][Wk2] Assertion failure when selecting an option in select list w…

…ith size attribute greater than one https://bugs.webkit.org/show_bug.cgi?id=86974 Patch by Dinu Jacob <dinu.jacob@nokia.com> on 2012-05-23 Reviewed by Simon Hausmann. Select list with size attribute greater than one will not initially have any item in selected state (if no option has 'selected' tag), resulting in m_selectedModelIndex in WebPopupMenuProxyQt to be invalid. Hence, need to check whether the old index is invalid before accessing the item at that index. * UIProcess/API/qt/tests/qmltests/DesktopBehavior/tst_itemSelector.qml: Added new test that tests selection in a select list with size attribute value of 2. * UIProcess/API/qt/tests/qmltests/common/selectwithsize.html: Added. * UIProcess/qt/WebPopupMenuProxyQt.cpp: (WebKit::PopupMenuItemModel::select): Check whether old index is valid before accessing the item at that index. Canonical link: https://commits.webkit.org/105025@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@118228 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · May 23, 2012 · 0bb267d · 0bb267d
1 parent 6a8a511
commit 0bb267d
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 3 deletions.
diff --git a/Source/WebKit2/ChangeLog b/Source/WebKit2/ChangeLog
@@ -1,3 +1,22 @@
+2012-05-23  Dinu Jacob  <dinu.jacob@nokia.com>
+
+        [Qt][Wk2] Assertion failure when selecting an option in select list with size attribute greater than one
+        https://bugs.webkit.org/show_bug.cgi?id=86974
+
+        Reviewed by Simon Hausmann.
+
+        Select list with size attribute greater than one will not initially have any
+        item in selected state (if no option has 'selected' tag), resulting in
+        m_selectedModelIndex in WebPopupMenuProxyQt to be invalid. Hence, need to check
+        whether the old index is invalid before accessing the item at that index.
+
+        * UIProcess/API/qt/tests/qmltests/DesktopBehavior/tst_itemSelector.qml: Added new test that
+          tests selection in a select list with size attribute value of 2.
+        * UIProcess/API/qt/tests/qmltests/common/selectwithsize.html: Added.
+        * UIProcess/qt/WebPopupMenuProxyQt.cpp:
+        (WebKit::PopupMenuItemModel::select): Check whether old index is valid before accessing
+          the item at that index.
+
 2012-05-23  Jer Noble  <jer.noble@apple.com>
 
         REGRESSION (r116188): After exiting full screen, Safari window is frozen, then inline video speeds through frames as it catches up with audio

diff --git a/Source/WebKit2/UIProcess/API/qt/tests/qmltests/DesktopBehavior/tst_itemSelector.qml b/Source/WebKit2/UIProcess/API/qt/tests/qmltests/DesktopBehavior/tst_itemSelector.qml
@@ -107,5 +107,13 @@ TestWebView {
             tryCompare(webView, "selectorLoaded", true)
             compare(webView.title, "No new selection was made")
         }
+
+        function test_selectWithSize() {
+            webView.url = Qt.resolvedUrl("../common/selectwithsize.html")
+            verify(webView.waitForLoadSucceeded())
+            titleSpy.clear()
+
+            test_selectFirstThenAcceptDirectly()
+        }
     }
 }
diff --git a/Source/WebKit2/UIProcess/API/qt/tests/qmltests/common/selectwithsize.html b/Source/WebKit2/UIProcess/API/qt/tests/qmltests/common/selectwithsize.html
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>No new selection was made</title>
+<script>
+function updateTitle(selectElement) {
+    var index = selectElement.selectedIndex;
+    document.title = selectElement.options[index].value;
+}
+</script>
+</head>
+<body>
+<select size=2 onchange="updateTitle(this)">
+<option value="__open__" >Open</option>
+<option value="__closed__" >Closed</option>
+<option value="__all__" >All</option>
+</select>
+</html>
diff --git a/Source/WebKit2/UIProcess/qt/WebPopupMenuProxyQt.cpp b/Source/WebKit2/UIProcess/qt/WebPopupMenuProxyQt.cpp
@@ -186,12 +186,15 @@ void PopupMenuItemModel::select(int index)
     if (!item.enabled)
         return;
 
-    Item& oldItem = m_items[oldIndex];
-    oldItem.selected = false;
     item.selected = true;
     m_selectedModelIndex = index;
 
-    emit dataChanged(this->index(oldIndex), this->index(oldIndex));
+    if (oldIndex != -1) {
+        Item& oldItem = m_items[oldIndex];
+        oldItem.selected = false;
+        emit dataChanged(this->index(oldIndex), this->index(oldIndex));
+    }
+
     emit dataChanged(this->index(index), this->index(index));
 }