[webkit-reviews] review denied: [Bug 34210] [Qt] Display page loading progress inside the QtLauncher location bar : [Attachment 47514] Patch

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has denied Andreas Kling
<andreas.kling at nokia.com>'s request for review:
Bug 34210: [Qt] Display page loading progress inside the QtLauncher location
bar
https://bugs.webkit.org/show_bug.cgi?id=34210

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

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>

> +class LocationEdit : public QLineEdit {
> +    Q_OBJECT
> +public:
> +    LocationEdit(QWidget *parent = 0)

Coding style: * placement is wrong.

> +protected:
> +    virtual void paintEvent(QPaintEvent *event)

Ditto.


> +#ifndef Q_WS_MAEMO_5
>	   statusBar()->showMessage(link);
> +#endif
>  #ifndef QT_NO_TOOLTIP
>	   if (!toolTip.isEmpty())
>	       QToolTip::showText(QCursor::pos(), toolTip);
> @@ -410,7 +453,9 @@ protected slots:
>	       QWebElementCollection result = 
view->page()->mainFrame()->findAllElements(str);
>	       foreach (QWebElement e, result)
>		   e.setStyleProperty("background-color", "yellow");
> +#ifndef Q_WS_MAEMO_5
>	       statusBar()->showMessage(QString("%1 element(s)
selected").arg(result.count()), 5000);
> +#endif

While I agree with the above two hunks (the flicker in the status bar is
annoying), but technically that's unrelated to the progress bar stuff, no?

I suggest to mention it at least in the changelog.


The rest looks good to me :)