[webkit-reviews] review denied: [Bug 11617] Compile and work on Qt/KDE platforms : [Attachment 11539] fixes the style issues

bugzilla-request-daemon at macosforge.org bugzilla-request-daemon at macosforge.org
Thu Nov 16 14:14:49 PST 2006

mitz at webkit.org has denied mitz at webkit.org's request for review:
Bug 11617: Compile and work on Qt/KDE platforms

Attachment 11539: fixes the style issues

------- Additional Comments from mitz at webkit.org
r- since there are still a few coding style and comment issues, but other than
that the patch looks good.

Please avoid trailing spaces:


Don't use tabs in ChangeLog:

+	http://bugs.webkit.org/show_bug.cgi?id=11617

+	Adjusting to the new api.

I don't think this comment makes sense for Qt:

+    // Double-click events don't exist in Cocoa. Since
passWidgetMouseDownEventToWidget will
+    // just pass currentEvent down to the widget, we don't want to call it for
events that
+    // don't correspond to Cocoa events.  The mousedown/ups will have already
been passed on as
+    // part of the pressed/released handling.

What's the point of this comment?

+    // FIXME: this method always returns true
+    notImplemented();
+    return false;

This should be '#include "Shared.h"'

+#include <Shared.h>

Should use braces for one-liners:

+    if (!m_menu) {
+	 m_menu = new QMenu();
+    }

FIXME style is "// FIXME: Sentence-capitalized comment". I think these comments
could be phrased better (the second one might not make sense once the first one
is changed or removed):

+    //FIXME this method is silly
+    //FIXME another silly method

Include <WTF/Forward.h> and drop the WTF:: from these:

+    virtual void
+    virtual void

More information about the webkit-reviews mailing list