[Webkit-unassigned] [Bug 85527] [Qt][WK2] Add support for multi-select list

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=85527





--- Comment #21 from Dinu Jacob <dinu.jacob at Nokia.com>  2012-05-31 19:07:22 PST ---
(In reply to comment #20)
> (From update of attachment 145120 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=145120&action=review
> 
> > Source/WebKit2/UIProcess/WebPageProxy.cpp:2759
> > +#if PLATFORM(QT)
> 
> Could those be in WebPageProxyQt.cpp maybe ?
> 
> > Source/WebKit2/UIProcess/qt/WebPopupMenuProxyQt.cpp:246
> > +    qSort(list);
> 
> In general, couldn't we avoid this whole logic of determining this selectedOriginalIndex() in the UI process at that point and passing it along with the HidePopupMenu messgage ?
> 
> Why not update the Button's text directly in the WebProcess ?
> 
> Seems to me that something like this should work:
> void RenderMenuList::listBoxSelectItem(int listIndex, bool allowMultiplySelections, bool shift, bool fireOnChangeNow)
> {
>     HTMLSelectElement* select = toHTMLSelectElement(node());
>     select->listBoxSelectItem(listIndex, allowMultiplySelections, shift, fireOnChangeNow);
> #if PLATFORM(QT)
>     const int firstSelectedIndex = select->selectedIndex();
>     if (listIndex <= firstSelectedIndex)
>         setTextFromItem(firstSelectedIndex);
> #endif
> }
> 
> With the added benefit of updating the combo button directly during the selection process.

Yes, I think this is much cleaner. This will eliminate having to maintain the list of selected indices as well.
> 
> > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2344
> > +#if PLATFORM(QT)
> 
> Same as above. WebPageQt.cpp ?


Didn't realize there was a Qt specific file for WebPage.cpp and WebPageProxy.cpp. Will move the changes there.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.