Bug 35877 – [Qt] [ rubberstamp!] Fix if statement in Qt Launcher

• Component: WebKit Qt
• Status: RESOLVED FIXED

[Robert Hogan]

Whew, had no luck in getting this rubberstamped, people are busy!

From fbee4aa7f643ba7ddb9729b82ea33ba1c47f1195 Mon Sep 17 00:00:00 2001                       
From: Robert Hogan <[e-mail]>                                                       
Date: Sun, 7 Mar 2010 12:44:44 +0000                                                         
Subject: [PATCH] 2010-03-07  Robert Hogan  <[e-mail]>                               

        Reviewed by NOBODY (OOPS!).

        QtLauncher: Fix typo in conditional statement in
                    WebViewGraphicsBased::setFrameRateMeasurementEnabled.

        Reviewed by:

        '=' should be '=='!

        * QtLauncher/webview.cpp:
        (WebViewGraphicsBased::setFrameRateMeasurementEnabled):
---
 WebKitTools/ChangeLog              |   14 ++++++++++++++
 WebKitTools/QtLauncher/webview.cpp |    2 +-
 2 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index 6efbd72..befeab7 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,17 @@
[phone]  Robert Hogan  <[e-mail]>
+
+        Reviewed by NOBODY (OOPS!).
+
+        QtLauncher: Fix typo in conditional statement in
+                    WebViewGraphicsBased::setFrameRateMeasurementEnabled.
+
+        Reviewed by:
+
+        '=' should be '=='!
+
+        * QtLauncher/webview.cpp:
+        (WebViewGraphicsBased::setFrameRateMeasurementEnabled):
+
 2010-03-08  Brady Eidson  <[e-mail]>

         Reviewed by NOBODY (but suggested by Steve Falkenburg and fixing a boneheaded mistake on my part last week)
diff --git a/WebKitTools/QtLauncher/webview.cpp b/WebKitTools/QtLauncher/webview.cpp
index 9252c89..ba1c9b8 100644
--- a/WebKitTools/QtLauncher/webview.cpp
+++ b/WebKitTools/QtLauncher/webview.cpp
@@ -87,7 +87,7 @@ void WebViewGraphicsBased::resizeEvent(QResizeEvent* event)

 void WebViewGraphicsBased::setFrameRateMeasurementEnabled(bool enabled)
 {
-    if (m_measureFps = enabled) {
+    if (m_measureFps == enabled) {
         m_lastConsultTime = m_startTime = QTime::currentTime();
         m_updateTimer->start();
     } else
--
1.6.3.3

[Robert Hogan]

Created attachment 50245 [details]
Patch

[Tor Arne Vestbø]

Please follow the QtWebKit bug reporting guidelines when reporting bugs.

See http://trac.webkit.org/wiki/QtWebKitBugs

Specifically:

  - The 'QtWebKit' component should only be used for bugs/features in the
    public QtWebKit API layer, not to signify that the bug is specific to
    the Qt port of WebKit

      http://trac.webkit.org/wiki/QtWebKitBugs#Component

  - Add the keyword 'Qt' to signal that it's a Qt-related bug

      http://trac.webkit.org/wiki/QtWebKitBugs#Keywords

[Robert Hogan]

Landed as http://trac.webkit.org/changeset/55791.

[Kenneth Rohde Christiansen]

 void WebViewGraphicsBased::setFrameRateMeasurementEnabled(bool enabled)
 {
-    if (m_measureFps = enabled) {
+    if (m_measureFps == enabled) {
         m_lastConsultTime = m_startTime = QTime::currentTime();
         m_updateTimer->start();
     } else

This patch is wrong and broke the feature of the launcher.

It attributes enabled to m_messureFpt and then does the if check.

This is the same as 

m_measureFps = enabled;

if (m_measureFps)...

Please revert this patch.

[Robert Hogan]

Created attachment 50624 [details]
Updated Patch

Patch was incorrect as Kenneth pointed out. Make it more obvious that it's already right!

[Adam Barth]

Comment on attachment 50245 [details]
Patch

Looks like this patch is obsolete.

[Robert Hogan]

Manually committed as: http://trac.webkit.org/changeset/55961