[webkit-reviews] review denied: [Bug 35755] [Qt] QtLauncher needs an option to toggle FullScreen Mode : [Attachment 50038] Add FullScreen option to menu

[bugzilla-daemon at webkit.org]

Ariya Hidayat <ariya.hidayat at gmail.com> has denied Jesus Sanchez-Palencia
<jesus.palencia at openbossa.org>'s request for review:
Bug 35755: [Qt] QtLauncher needs an option to toggle FullScreen Mode
https://bugs.webkit.org/show_bug.cgi?id=35755

Attachment 50038: Add FullScreen option to menu
https://bugs.webkit.org/attachment.cgi?id=50038&action=review

------- Additional Comments from Ariya Hidayat <ariya.hidayat at gmail.com>
Overall looks good.

> +signals:
> +    void fullscreen(bool on);

Usually signal name is future (aboutToDoFoo) or past (foobarHasChanged).

> +    // If click pos is the bottom right corner (square with size defined by
gExitClickArea)
> +    // and the window is on FullScreen, the window must return to its
original state.
> +    if (event->type() == QEvent::MouseButtonRelease) {
> +	   QMouseEvent* ev = static_cast<QMouseEvent*>(event);
> +	   if (windowState() == Qt::WindowFullScreen
> +	       && ev->pos().x() > (size().width() - gExitClickArea)
> +	       && ev->pos().y() > (size().height() - gExitClickArea)) {

Using width() and height() directly does not work?

> +    toggleFullScreen->connect(this, SIGNAL(fullscreen(bool)),
SLOT(setChecked(bool)));

Is this a correct connection?