[webkit-reviews] review denied: [Bug 62863] In QWebFrame, m_orientation is started but never stopped : [Attachment 97603] remove unnecessary #include from original patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jun 17 09:06:26 PDT 2011


Andreas Kling <kling at webkit.org> has denied Fabrizio
<fabrizio.machado at nokia.com>'s request for review:
Bug 62863: In QWebFrame, m_orientation is started but never stopped
https://bugs.webkit.org/show_bug.cgi?id=62863

Attachment 97603: remove unnecessary #include from original patch
https://bugs.webkit.org/attachment.cgi?id=97603&action=review

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

What about QGraphicsWebView?
And this feels very hackish, I don't think all users of Q*WebView would want
the orientation events to disappear when the app is in the background.
Perhaps we could have an explicit API (on QWebFrame) to enable/disable the
accelerometer? I'd probably \internal that kind of API though.

> Source/WebKit/qt/Api/qwebview.cpp:830
> +	   QWebFrame* frame = page()->mainFrame();

You should use d->page instead of page() here.

> Source/WebKit/qt/Api/qwebview.cpp:834
> +	   if (e->type() == QEvent::WindowDeactivate)
> +		 frame->d->m_orientation.stop();
> +	   if (e->type() == QEvent::WindowActivate)
> +		 frame->d->m_orientation.start();

Coding style, tabs are 4 spaces. Also, second condition should be 'else if'.


More information about the webkit-reviews mailing list