Bug 86974

Summary: [Qt][Wk2] Assertion failure when selecting an option in select list with size attribute greater than one
Product: WebKit Reporter: Dinu Jacob <dinu.jacob>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, menard, webkit.review.bot, zoltan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Dinu Jacob 2012-05-20 16:13:19 PDT 
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]
Comment 1 Dinu Jacob 2012-05-20 16:30:15 PDT 
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.
Comment 2 Dinu Jacob 2012-05-20 16:33:22 PDT 
Created attachment 142925 [details]
Patch
Comment 3 Caio Marcelo de Oliveira Filho 2012-05-22 19:11:26 PDT 
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.
Comment 4 Dinu Jacob 2012-05-23 07:04:27 PDT 
Created attachment 143562 [details]
Patch
Comment 5 Dinu Jacob 2012-05-23 07:06:12 PDT 
(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.
Comment 6 WebKit Review Bot 2012-05-23 12:43:12 PDT 
Comment on attachment 143562 [details]
Patch

Clearing flags on attachment: 143562

Committed r118228: <http://trac.webkit.org/changeset/118228>
Comment 7 WebKit Review Bot 2012-05-23 12:43:17 PDT 
All reviewed patches have been landed.  Closing bug.