Bug 27772 – PopupMenuQt lacks disabled and selected support

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Mike Fenton]

PopupMenuQt previously had a code block defined out that was meant to provide the ability to disable entries and select the current entry.

This functionality should be re-instated.

[Mike Fenton]

Created attachment 33658 [details]
Patch that enables selected and disabled support for PopupMenuQt.

[Adam Treat]

Comment on attachment 33658 [details]
Patch that enables selected and disabled support for PopupMenuQt.

> 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?

[Mike Fenton]

Created attachment 33795 [details]
Patch with updated style / model check.

Thanks for the review.   Patch is updated as suggested.

[Adam Treat]

Comment on attachment 33795 [details]
Patch with updated style / model check.

> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> +
> +            if (model && !client()->itemIsEnabled(i))
> +              model->item(i)->setEnabled(false);
> +
> +            if (client()->itemIsSelected(i))
> +                m_popup->setCurrentIndex(i);
> +        }

Indentation is not quite right, but I'll fix it when I land.  Otherwise r=me!

[Adam Treat]

Landed with r46593.

[Simon Hausmann]

*** Bug 23712 has been marked as a duplicate of this bug. ***