[webkit-reviews] review denied: [Bug 27772] PopupMenuQt lacks disabled and selected support : [Attachment 33658] Patch that enables selected and disabled support for PopupMenuQt.

[bugzilla-daemon at webkit.org]

Adam Treat <treat at kde.org> has denied Mike Fenton
<mike.fenton at torchmobile.com>'s request for review:
Bug 27772: PopupMenuQt lacks disabled and selected support
https://bugs.webkit.org/show_bug.cgi?id=27772

Attachment 33658: Patch that enables selected and disabled support for
PopupMenuQt.
https://bugs.webkit.org/attachment.cgi?id=33658&action=review

------- Additional Comments from Adam Treat <treat at kde.org>
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> +    QStandardItemModel *model =
qobject_cast<QStandardItemModel*>(m_popup->model());
> +    Q_ASSERT(model);

Style decoration problem.  See coding guidelines.

> +	       if (client()->itemIsSelected(i))
> +		   m_popup->setCurrentIndex(i);

The big fear I have is that QComboBox uses QStandardItemModel internally... if
they change that to using something else we'll be asserting.

How about this... Still ASSERT on the model, but further down replace with:

    if (model && !client()->itemIsEnabled())

That way we won't worry in release builds.

Sound ok?