Bug 86974 – [Qt][Wk2] Assertion failure when selecting an option in select list with size attribute greater than one

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Dinu Jacob]

Assertion failure when selecting an option from a select list that has the size attribute set to a value greater than one:

<select id="sel" name="cars" size=2 >
<option value="volvo" selected="selected">Volvo</option>
<option value="ford" >Ford</option>
<option value="toyota">Toyota   </option>
</select>


ASSERTION FAILED: i < size()
Source/WTF/wtf/Vector.h(527) : T& WTF::Vector<T, inlineCapacity>::at(size_t) [with T = WebKit::PopupMenuItemModel::Item, unsigned int inlineCapacity = 0u, size_t = unsigned int]
1   0xb72398e4 WebKitBuild/Debug/lib/libWebKit2.so.1(_ZN3WTF6VectorIN6WebKit18PopupMenuItemModel4ItemELj0EE2atEj+0x56) [0xb72398e4]
2   0xb723942c WebKitBuild/Debug/lib/libWebKit2.so.1(_ZN3WTF6VectorIN6WebKit18PopupMenuItemModel4ItemELj0EEixEj+0x24) [0xb723942c]
3   0xb7237f1c WebKitBuild/Debug/lib/libWebKit2.so.1(_ZN6WebKit18PopupMenuItemModel6selectEi+0x98) [0xb7237f1c]
4   0xb7237a3b WebKitBuild/Debug/lib/libWebKit2.so.1(_ZN6WebKit25ItemSelectorContextObject6acceptEi+0x2d) [0xb7237a3b]
5   0xb7238db9 WebKitBuild/Debug/lib/libWebKit2.so.1(+0x6cddb9) [0xb7238db9]
6   0xb7238f66 WebKitBuild/Debug/lib/libWebKit2.so.1(_ZN6WebKit25ItemSelectorContextObject11qt_metacallEN11QMetaObject4CallEiPPv+0x70) [0xb7238f66]

[Dinu Jacob]

Select list with size attribute value of greater than one and no option pre-selected will not have any option in selected state initially. This will cause m_selectedModelIndex to be -1. On selecting an option from the list will result in accessing an item from the list of options with this invalid index resulting in the assertion.

[Dinu Jacob]

Created attachment 142925 [details]
Patch

[Caio Marcelo de Oliveira Filho]

Comment on attachment 142925 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=142925&action=review

Nice catch! Looks good to me, with a comment.

> Source/WebKit2/UIProcess/qt/WebPopupMenuProxyQt.cpp:192
> +        emit dataChanged(this->index(oldIndex), this->index(oldIndex));

I think is better to keep the signal emission where it was, after we update the new selected item. This way, if someone reacts to the dataChanged, our model will be in a consistent state.

[Dinu Jacob]

Created attachment 143562 [details]
Patch

[Dinu Jacob]

(In reply to comment #3)
> (From update of attachment 142925 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=142925&action=review
> 
> Nice catch! Looks good to me, with a comment.
> 
> > Source/WebKit2/UIProcess/qt/WebPopupMenuProxyQt.cpp:192
> > +        emit dataChanged(this->index(oldIndex), this->index(oldIndex));
> 
> I think is better to keep the signal emission where it was, after we update the new selected item. This way, if someone reacts to the dataChanged, our model will be in a consistent state.

Changed to emit the signals after updating the states.

[WebKit Review Bot]

Comment on attachment 143562 [details]
Patch

Clearing flags on attachment: 143562

Committed r118228: <http://trac.webkit.org/changeset/118228>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.