[webkit-reviews] review denied: [Bug 58188] [Qt] MediaPlayerPrivateQt::supportsType does not parse codec parameter : [Attachment 89030] Modified patch based on Benjamin's comments

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has denied Nancy Piedra
<nancy.piedra at nokia.com>'s request for review:
Bug 58188: [Qt] MediaPlayerPrivateQt::supportsType does not parse codec
parameter
https://bugs.webkit.org/show_bug.cgi?id=58188

Attachment 89030: Modified patch based on Benjamin's comments
https://bugs.webkit.org/attachment.cgi?id=89030&action=review

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=89030&action=review

I am ok with the change. It is not very solid but hey, that is the way
QMediaPlayer is defined.
Just clean the patch before it can land.

> Source/WebCore/ChangeLog:9
> +	   a QStringList.  This change parses and trims the list.

Two spaces after the dot.

> Source/WebCore/platform/graphics/qt/MediaPlayerPrivateQt.cpp:91
> +    foreach (const QString &codecStrNotTrimmed, codecList) {

coding style: 
const QString& codecStrNotTrimmed.

> Source/WebCore/platform/graphics/qt/MediaPlayerPrivateQt.cpp:93
> +	   codecListTrimmed.append(codecStrTrimmed);

Couldn't you have an empty string here.
If codecs = "foo,    , bar,    ". The parts containing only spaces will be null
string after trimmed() and will be inserted in the list.