Commit d465668b authored by bweinstein@apple.com's avatar bweinstein@apple.com

Crash in WebKit!WebCore::RenderMenuList::itemStyle

https://bugs.webkit.org/show_bug.cgi?id=34182
<rdar://7087757>
        
Reviewed by Jon Honeycutt.

Added bounds checks in RenderMenuList to make sure we are
not making an out of bounds check in a vector once an option
element has been deleted. If we are out of bounds, we fall back to
a default value and return early, and in the case of itemStyle, we use a 
previous option's style, if it is available.

* manual-tests/select-delete-item.html: Added.
* rendering/RenderMenuList.cpp:
(WebCore::RenderMenuList::itemText): If out of bounds check, return early.
(WebCore::RenderMenuList::itemToolTip): Ditto.
(WebCore::RenderMenuList::itemIsEnabled): Ditto.
(WebCore::RenderMenuList::itemStyle): If out of bounds check, try using the 0th index
    option style, then fall back to the select's style if that option doesn't exist.
(WebCore::RenderMenuList::itemBackgroundColor): If out of bounds check, return early.
(WebCore::RenderMenuList::itemIsSeparator): Ditto.
(WebCore::RenderMenuList::itemIsLabel): Ditto.
(WebCore::RenderMenuList::itemIsSelected): Ditto.



git-svn-id: svn://svn.chromium.org/blink/trunk@53867 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent b2c2f9cb
2010-01-26 Brian Weinstein <bweinstein@apple.com>
Reviewed by Jon Honeycutt.
Crash in WebKit!WebCore::RenderMenuList::itemStyle
https://bugs.webkit.org/show_bug.cgi?id=34182
<rdar://7087757>
Added bounds checks in RenderMenuList to make sure we are
not making an out of bounds check in a vector once an option
element has been deleted. If we are out of bounds, we fall back to
a default value and return early, and in the case of itemStyle, we use a
previous option's style, if it is available.
* manual-tests/select-delete-item.html: Added.
* rendering/RenderMenuList.cpp:
(WebCore::RenderMenuList::itemText): If out of bounds check, return early.
(WebCore::RenderMenuList::itemToolTip): Ditto.
(WebCore::RenderMenuList::itemIsEnabled): Ditto.
(WebCore::RenderMenuList::itemStyle): If out of bounds check, try using the 0th index
option style, then fall back to the select's style if that option doesn't exist.
(WebCore::RenderMenuList::itemBackgroundColor): If out of bounds check, return early.
(WebCore::RenderMenuList::itemIsSeparator): Ditto.
(WebCore::RenderMenuList::itemIsLabel): Ditto.
(WebCore::RenderMenuList::itemIsSelected): Ditto.
2010-01-25 Gavin Barraclough <barraclough@apple.com>
Reviewed by Anders Carlsson.
<html>
<head>
<title>RenderMenuList::itemStyle Select Element Crash</title>
<script>
function removeItem() {
var select = document.getElementById("dropDown");
select.removeChild(document.getElementsByTagName("option")[2]);
}
</script>
</head>
<body>
<select id="dropDown" onfocus="setTimeout('removeItem();', 2000);">
<option>Option 1</option>
<option>Option 2</option>
<option>Option 3</option>
</select>
<p>This is a test for bug <a href="http://webkit.org/b/34182">34182</a> Crash in WebKit!WebCore::RenderMenuList::itemStyle.
Once the select gets focus, in 2 seconds it will delete an item. This test passes
if you have the select open when it deletes an item, and doesn't crash.</p>
</body>
</html>
......@@ -323,7 +323,10 @@ void RenderMenuList::didSetSelectedIndex()
String RenderMenuList::itemText(unsigned listIndex) const
{
SelectElement* select = toSelectElement(static_cast<Element*>(node()));
Element* element = select->listItems()[listIndex];
const Vector<Element*>& listItems = select->listItems();
if (listIndex >= listItems.size())
return String();
Element* element = listItems[listIndex];
if (OptionGroupElement* optionGroupElement = toOptionGroupElement(element))
return optionGroupElement->groupLabelText();
else if (OptionElement* optionElement = toOptionElement(element))
......@@ -334,14 +337,20 @@ String RenderMenuList::itemText(unsigned listIndex) const
String RenderMenuList::itemToolTip(unsigned listIndex) const
{
SelectElement* select = toSelectElement(static_cast<Element*>(node()));
Element* element = select->listItems()[listIndex];
const Vector<Element*>& listItems = select->listItems();
if (listIndex >= listItems.size())
return String();
Element* element = listItems[listIndex];
return element->title();
}
bool RenderMenuList::itemIsEnabled(unsigned listIndex) const
{
SelectElement* select = toSelectElement(static_cast<Element*>(node()));
Element* element = select->listItems()[listIndex];
const Vector<Element*>& listItems = select->listItems();
if (listIndex >= listItems.size())
return false;
Element* element = listItems[listIndex];
if (!isOptionElement(element))
return false;
......@@ -359,7 +368,18 @@ bool RenderMenuList::itemIsEnabled(unsigned listIndex) const
PopupMenuStyle RenderMenuList::itemStyle(unsigned listIndex) const
{
SelectElement* select = toSelectElement(static_cast<Element*>(node()));
Element* element = select->listItems()[listIndex];
const Vector<Element*>& listItems = select->listItems();
if (listIndex >= listItems.size()) {
// If we are making an out of bounds access, then we want to use the style
// of a different option element (index 0). However, if there isn't an option element
// before at index 0, we fall back to the menu's style.
if (!listIndex)
return menuStyle();
// Try to retrieve the style of an option element we know exists (index 0).
listIndex = 0;
}
Element* element = listItems[listIndex];
RenderStyle* style = element->renderStyle() ? element->renderStyle() : element->computedStyle();
return style ? PopupMenuStyle(style->color(), itemBackgroundColor(listIndex), style->font(), style->visibility() == VISIBLE, style->textIndent(), style->direction()) : menuStyle();
......@@ -368,7 +388,10 @@ PopupMenuStyle RenderMenuList::itemStyle(unsigned listIndex) const
Color RenderMenuList::itemBackgroundColor(unsigned listIndex) const
{
SelectElement* select = toSelectElement(static_cast<Element*>(node()));
Element* element = select->listItems()[listIndex];
const Vector<Element*>& listItems = select->listItems();
if (listIndex >= listItems.size())
return style()->backgroundColor();
Element* element = listItems[listIndex];
Color backgroundColor;
if (element->renderStyle())
......@@ -459,21 +482,30 @@ void RenderMenuList::popupDidHide()
bool RenderMenuList::itemIsSeparator(unsigned listIndex) const
{
SelectElement* select = toSelectElement(static_cast<Element*>(node()));
Element* element = select->listItems()[listIndex];
const Vector<Element*>& listItems = select->listItems();
if (listIndex >= listItems.size())
return false;
Element* element = listItems[listIndex];
return element->hasTagName(hrTag);
}
bool RenderMenuList::itemIsLabel(unsigned listIndex) const
{
SelectElement* select = toSelectElement(static_cast<Element*>(node()));
Element* element = select->listItems()[listIndex];
const Vector<Element*>& listItems = select->listItems();
if (listIndex >= listItems.size())
return false;
Element* element = listItems[listIndex];
return isOptionGroupElement(element);
}
bool RenderMenuList::itemIsSelected(unsigned listIndex) const
{
SelectElement* select = toSelectElement(static_cast<Element*>(node()));
Element* element = select->listItems()[listIndex];
const Vector<Element*>& listItems = select->listItems();
if (listIndex >= listItems.size())
return false;
Element* element = listItems[listIndex];
if (OptionElement* optionElement = toOptionElement(element))
return optionElement->selected();
return false;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment