[webkit-reviews] review denied: [Bug 58435] [Qt] Upstream Symbian platform plugin. : [Attachment 91542] platformpluginV21

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue May 3 15:17:53 PDT 2011


Alexis Menard <alexis.menard at openbossa.org> has denied yi shen
<yi.4.shen at nokia.com>'s request for review:
Bug 58435: [Qt] Upstream Symbian platform plugin.
https://bugs.webkit.org/show_bug.cgi?id=58435

Attachment 91542: platformpluginV21
https://bugs.webkit.org/attachment.cgi?id=91542&action=review

------- Additional Comments from Alexis Menard <alexis.menard at openbossa.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=91542&action=review

> Source/WebKit/qt/symbian/platformplugin/OverlayWidget.cpp:30
> +    : QWidget(parent, Qt::Tool | Qt::FramelessWindowHint)

I'm sure the button does not show up because of the parent (or something like
that).

> Source/WebKit/qt/symbian/platformplugin/OverlayWidget.cpp:45
> +    m_slider = new QSlider(Qt::Horizontal, this);

better name?

> Source/WebKit/qt/symbian/platformplugin/PlayerLabel.cpp:32
> +    m_bufferingAnimationIterator = m_bufferingAnimation;

Can you just take one icon and rotate it while you render? (with different
angle).

> Source/WebKit/qt/symbian/platformplugin/WebPlugin.cpp:23
> +#if defined(ENABLE_QT_MULTIMEDIA) && ENABLE_QT_MULTIMEDIA

Please be consistent with trunk. USE.

> Source/WebKit/qt/symbian/platformplugin/WebPlugin.cpp:254
> +#if defined(ENABLE_QT_MULTIMEDIA) && ENABLE_QT_MULTIMEDIA

Ditto.

> Source/WebKit/qt/symbian/platformplugin/platformplugin.pro:22
> +    DEFINES += ENABLE_QT_MULTIMEDIA=1

Get rid of that one and update the code of the plugin.


More information about the webkit-reviews mailing list