[Webkit-unassigned] [Bug 15682] Move wx port to TOT

[bugzilla-daemon at webkit.org]

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


mrowe at apple.com changed:

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




------- Comment #62 from mrowe at apple.com  2007-11-08 15:05 PDT -------
(From update of attachment 17065)
Several of the classes (wxWebView, wxWebViewDOMElementInfo, and others) have
getter methods that should be declared as const.

wxWebView's m_impl member should not be public.

The wxWEBVIEW_STATE_FAILED enum declaration looks to have some whacky
indentation.

We typically declare members as private rather than protected.

Several of your macros starting with EVT_WEBVIEW_STATE_CHANGED use C-style
casts rather than C++-style.

The wxPageSourceViewFrame constructor assigns to a variable named control which
is never used.  The variable declaration seems unnecessary.

Right above the declaration of ID_LOADFILE the { should be on the same line as
"enum".

The wxWebFrame constructor should initialize m_checkBeforeLoad in it's
initialization list.

I'm not sure that the API should create a status bar with the text "wxWebKit
works" in it :)

Many of the wxWebFrame methods have {} placement issues -- too many, and on the
wrong lines.

wxWebFrame::OnAbout has some extra whitespace inside function calls.

wxWebFrame::OnStop has weird indentation.

The message boxes in wxWebFrame::OnMakeTextLarger don't seem desirable for an
API.

wxWebViewDOMElementInfo and wxWebView should also use the initializer list
instead of assignments.

wxWebView::~wxWebView has a printf which should be removed.

wxWebView::m_impl may be better suited as an OwnPtr to avoid the need to
explicitly delete it in the destructor.

wxWebView::SetPageSource has some C-style casts.

The code in wxWebView::LoadURL checks whether a file exists on disk then uses
"ftp" for the protocol in that instance.  That looks incorrect.

wxWebView::OnKeyEvents has some more C-style casts.

Because of the issues above I'm going to say r- for now.  Many were coding
style issues, but it'd be good for an updated version of the patch to get
another look from someone else too.


-- 
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.