[webkit-reviews] review granted: [Bug 10466] WebKit should have Qt platform support : [Attachment 10104] Initial patch

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Fri Aug 18 01:41:55 PDT 2006


Eric Seidel <macdome at opendarwin.org> has granted Eric Seidel
<macdome at opendarwin.org>'s request for review:
Bug 10466: WebKit should have Qt platform support
http://bugzilla.opendarwin.org/show_bug.cgi?id=10466

Attachment 10104: Initial patch
http://bugzilla.opendarwin.org/attachment.cgi?id=10104&action=edit

------- Additional Comments from Eric Seidel <macdome at opendarwin.org>
This would probably be better as if !PLATFORM(QT)

+#if PLATFORM(QT)
+	 // use default QCursor constructor. QCursor(0) creates
+	 // an invalid cursor due to implicit constructors ;(
+#else
+	 : m_impl(0)
+#endif
+		{ }

Would be nice to fix the funny spacing while we're there:

+#if PLATFORM(GDK) || PLATFORM(QT)
     ResourceLoaderInternal * getInternal() { return d;}
 #endif

No need to name the argument "parent" here, it's implied by the method:

+#if PLATFORM(QT)
+    virtual void setParentWidget(QWidget* parent);
+#endif


Same here:

+#if PLATFORM(QT)
+	 QWidget* parentWidget() const;
+	 virtual void setParentWidget(QWidget* parent);
+
+	 QWidget* qwidget();
+	 void setQWidget(QWidget* widget);
+#endif

named arguments in headers often tend to just clutter things.

and another:

+
+	 virtual void setParentWidget(QWidget* parent);
+

This is a really nice patch though.

r=me



More information about the webkit-reviews mailing list