[Webkit-unassigned] [Bug 11617] Compile and work on Qt/KDE platforms

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


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


mitz at webkit.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #11539|review?                     |review-
               Flag|                            |




------- Comment #4 from mitz at webkit.org  2006-11-16 14:14 PDT -------
(From update of attachment 11539)
r- since there are still a few coding style and comment issues, but other than
that the patch looks good.

Please avoid trailing spaces:

-   ${CMAKE_CURRENT_SOURCE_DIR}/page
+   ${CMAKE_CURRENT_SOURCE_DIR}/page   

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
registerCommandForUndo(WTF::PassRefPtr<WebCore::EditCommand>);
+    virtual void
registerCommandForRedo(WTF::PassRefPtr<WebCore::EditCommand>);


-- 
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.



More information about the webkit-unassigned mailing list