[webkit-reviews] review denied: [Bug 52010] [Qt] Plugin API to support platform's input widgets. : [Attachment 78185] Patch 01

[bugzilla-daemon at webkit.org]

Andreas Kling <kling at webkit.org> has denied Ragner Magalhaes
<ragner.magalhaes at openbossa.org>'s request for review:
Bug 52010: [Qt] Plugin API to support platform's input widgets.
https://bugs.webkit.org/show_bug.cgi?id=52010

Attachment 78185: Patch 01
https://bugs.webkit.org/attachment.cgi?id=78185&action=review

------- Additional Comments from Andreas Kling <kling at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=78185&action=review

I really don't like using strings for determining the tag names and input
types, can we extend QWebElement to expose the WebCore InputType somehow?

> WebKit/qt/Api/qwebkitplatformplugin.h:122
> +    virtual ~QWebInputView() {}

Coding style, {} -> { }

> WebKit/qt/Api/qwebkitplatformplugin.h:125
> +    virtual void setWebElement(QWebElement) = 0;

QWebElement -> const QWebElement&

> WebKit/qt/WebCoreSupport/ChromeClientQt.cpp:691
> +    if (element.isNull() || element.tagName() != "INPUT")

"INPUT" should be wrapped in a QLatin1String. Also, the tagName() may not be
all-uppercase, in non-HTML documents for example.

> WebKit/qt/WebCoreSupport/ChromeClientQt.cpp:696
> +    QWebInputView* result = 0;
> +
> +    result = m_platformPlugin.createInputView(element.attribute("type"));

No need for the temporary 0 assignment.
"type" should be wrapped in a QLatin1String.

> WebKit/qt/WebCoreSupport/ChromeClientQt.h:188
> +	   QWebInputView *createInputView(QWebElement) const;

QWebElement -> const QWebElement&