summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authortkent <tkent@chromium.org>2016-09-21 07:02:46 (GMT)
committerCommit bot <commit-bot@chromium.org>2016-09-21 07:06:18 (GMT)
commit676be06451fec4d4f2f0bab075f3b627ec1d1597 (patch)
tree5e41ec74635673a09f66d80222993a76c9c5da9c
parent0386453daf54e45a89e163f2d632b8864c9d3975 (diff)
downloadchromium-676be06451fec4d4f2f0bab075f3b627ec1d1597.tar.gz
chromium-676be06451fec4d4f2f0bab075f3b627ec1d1597.tar.xz
Fix a DCHECK failure in PopupMenuImpl::setValueAndClosePopup().
|stringValue| may be empty. BUG=629029 Review-Url: https://codereview.chromium.org/2350303005 Cr-Commit-Position: refs/heads/master@{#419991}
-rw-r--r--third_party/WebKit/LayoutTests/fast/forms/select-popup/popup-menu-key-operations.html18
-rw-r--r--third_party/WebKit/LayoutTests/platform/win/fast/forms/select-popup/popup-menu-key-operations-expected.txt5
-rw-r--r--third_party/WebKit/Source/web/PopupMenuImpl.cpp12
3 files changed, 30 insertions, 5 deletions
diff --git a/third_party/WebKit/LayoutTests/fast/forms/select-popup/popup-menu-key-operations.html b/third_party/WebKit/LayoutTests/fast/forms/select-popup/popup-menu-key-operations.html
index 54febb5..9287cbb 100644
--- a/third_party/WebKit/LayoutTests/fast/forms/select-popup/popup-menu-key-operations.html
+++ b/third_party/WebKit/LayoutTests/fast/forms/select-popup/popup-menu-key-operations.html
@@ -14,9 +14,14 @@
<option>garply</option>
</optgroup>
</select>
+<select id="menu2">
+ <option>abc</option>
+ <option>xyz</option>
+</select>
<div id="console"></div>
<script>
var menu = document.getElementById('menu');
+var menu2 = document.getElementById('menu2');
var picker = null;
function openPickerErrorCallback() {
testFailed('picker didn\'t open')
@@ -171,6 +176,19 @@ function test6() {
shouldBeEqualToString('menu.value', 'foo');
shouldBeEqualToString('internals.selectMenuListText(menu)', 'foo');
+ waitUntilClosing(() => {
+ menu2.selectedIndex = -1;
+ openPicker(menu2, testEnterWithoutSelection, openPickerErrorCallback);
+ });
+}
+
+function testEnterWithoutSelection() {
+ picker = window.internals.pagePopupWindow.global.picker;
+ shouldBeEqualToString('picker._selectElement.value', '');
+ eventSender.keyDown('Enter');
+ shouldBeNull('window.internals.pagePopupWindow');
+ shouldBeEqualToString('menu2.value', '');
+
finishJSTest();
}
</script>
diff --git a/third_party/WebKit/LayoutTests/platform/win/fast/forms/select-popup/popup-menu-key-operations-expected.txt b/third_party/WebKit/LayoutTests/platform/win/fast/forms/select-popup/popup-menu-key-operations-expected.txt
index c3aa5bc..4746a193e 100644
--- a/third_party/WebKit/LayoutTests/platform/win/fast/forms/select-popup/popup-menu-key-operations-expected.txt
+++ b/third_party/WebKit/LayoutTests/platform/win/fast/forms/select-popup/popup-menu-key-operations-expected.txt
@@ -1,4 +1,4 @@
-
+
PASS picker._selectElement.value is "1"
PASS menu.value is "bar"
PASS internals.selectMenuListText(menu) is "bar"
@@ -63,6 +63,9 @@ PASS internals.selectMenuListText(menu) is "bar"
PASS window.internals.pagePopupWindow is null
PASS menu.value is "foo"
PASS internals.selectMenuListText(menu) is "foo"
+PASS picker._selectElement.value is ""
+PASS window.internals.pagePopupWindow is null
+PASS menu2.value is ""
PASS successfullyParsed is true
TEST COMPLETE
diff --git a/third_party/WebKit/Source/web/PopupMenuImpl.cpp b/third_party/WebKit/Source/web/PopupMenuImpl.cpp
index af349b9..39d7b80 100644
--- a/third_party/WebKit/Source/web/PopupMenuImpl.cpp
+++ b/third_party/WebKit/Source/web/PopupMenuImpl.cpp
@@ -394,10 +394,11 @@ void PopupMenuImpl::setValueAndClosePopup(int numValue, const String& stringValu
{
DCHECK(m_popup);
DCHECK(m_ownerElement);
- bool success;
- int listIndex = stringValue.toInt(&success);
- DCHECK(success);
- {
+ if (!stringValue.isEmpty()) {
+ bool success;
+ int listIndex = stringValue.toInt(&success);
+ DCHECK(success);
+
EventQueueScope scope;
m_ownerElement->selectOptionByPopup(listIndex);
if (m_popup)
@@ -405,6 +406,9 @@ void PopupMenuImpl::setValueAndClosePopup(int numValue, const String& stringValu
// 'change' event is dispatched here. For compatbility with
// Angular 1.2, we need to dispatch a change event before
// mouseup/click events.
+ } else {
+ if (m_popup)
+ m_chromeClient->closePagePopup(m_popup);
}
// We dispatch events on the owner element to match the legacy behavior.
// Other browsers dispatch click events before and after showing the popup.