[Webkit-unassigned] [Bug 40718] [Qt] Platform plugin's multi-select does not take OptGroup into account

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Luiz Agostini <luiz.agostini at openbossa.org>  2010-06-17 09:24:28 PST ---
(From update of attachment 58953)
WebCore/dom/SelectElement.cpp:93
 +      if (!data.multiple() && data.size() <= 1) {
Why is it needed?

WebCore/rendering/RenderMenuList.cpp:317
 +      if (allowMultiplySelections)
This is not right. The combobox behavior must be based on it being multiple or not and not on any parameters of this method. Parameters allowMultiplySelections and shift must be ignored if the combobox is not null.

I beleave that the right thing to do is:

diff --git a/WebCore/html/HTMLSelectElement.cpp b/WebCore/html/HTMLSelectElement.cpp
index 9aacbb6..2dcac59 100644
--- a/WebCore/html/HTMLSelectElement.cpp
+++ b/WebCore/html/HTMLSelectElement.cpp
@@ -107,7 +107,7 @@ void HTMLSelectElement::setSelectedIndexByUser(int optionIndex, bool deselect, b
 void HTMLSelectElement::listBoxSelectItem(int listIndex, bool allowMultiplySelections, bool shift, bool fireOnChangeNow)
 {
     if (!multiple())
-        setSelectedIndexByUser(listIndex, true, fireOnChangeNow);
+        setSelectedIndexByUser(listToOptionIndex(listIndex), true, fireOnChangeNow);
     else {
         updateSelectedState(m_data, this, listIndex, allowMultiplySelections, shift);
         if (fireOnChangeNow)
diff --git a/WebCore/rendering/RenderMenuList.cpp b/WebCore/rendering/RenderMenuList.cpp
index 871c10f..77fe3c2 100644
--- a/WebCore/rendering/RenderMenuList.cpp
+++ b/WebCore/rendering/RenderMenuList.cpp
@@ -314,7 +314,7 @@ void RenderMenuList::valueChanged(unsigned listIndex, bool fireOnChange)
 void RenderMenuList::listBoxSelectItem(int listIndex, bool allowMultiplySelections, bool shift, bool fireOnChangeNow)
 {
     SelectElement* select = toSelectElement(static_cast<Element*>(node()));
-    select->listBoxSelectItem(select->listToOptionIndex(listIndex), allowMultiplySelections, shift, fireOnChangeNow);
+    select->listBoxSelectItem(listIndex, allowMultiplySelections, shift, fireOnChangeNow);
 }

WebKit/qt/WebCoreSupport/QtPlatformPlugin.h:51
 +      SelectData* m_selectData;
You can just declare m_selectData as a pointer to QWebSelectData instead of a pointer to SelectData. No memory will leak because none of those classes own any memory that should be released. But please help me with one big mistake I made: I declared inline the QWebSelectData's destructor and it should be virtual instead. The same thing for QWebSelectMethod. Could you please fix it?

A virtual destructor should be added to class QWebNotificationData as well.

Shouldn't you check if m_selectData is not null and destroy it before creating a new one in show().

WebKit/qt/examples/platformplugin/WebPlugin.h:36
 +      virtual bool isMultiple() const = 0;
This is not needed. Parameters allowMultipleSelections and shift will be ignored if the combobox is not multiple.

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