[Webkit-unassigned] [Bug 20393] Add WML support to WebKit

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Aug 16 09:03:19 PDT 2008


https://bugs.webkit.org/show_bug.cgi?id=20393


rwlbuis at gmail.com changed:

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




------- Comment #5 from rwlbuis at gmail.com  2008-08-16 09:03 PDT -------
(From update of attachment 22813)
I think in general this patch is too large as-is to review. Therefore I only
looked at coding style issues and not at what it exactly does. Maybe the patch
can be split up in wml specific, changes to the non-wml code and tests?

The coding style does not seem consistent, which is wrong per definition. I
assume the webkit style must be followed, which means all if(, for( and while(
uses should be if (, for ( and while (,
Here are some coding style issues:

Example of if( use where if ( should be used:

>+
>+void WMLAElement::defaultEventHandler(Event* evt) 
>+{
>+    bool shouldHandle = false;
>+    if(evt->type() == clickEvent) {
>+        shouldHandle = true;

Example of while( use where while ( should be used:

>+void WMLCardElement::registerDoElement(WMLDoElement* e)
>+{
>+    if(e) {
>+        Vector<WMLDoElement*>::iterator it = m_doVecs.begin();
>+        WMLDoElement* doe = NULL;
>+        while(it != m_doVecs.end()) {

Example of for( use where for ( should be used:

>+
>+    String templDoName;
>+    String cardDoName;
>+    WMLDoElement* templDo = 0;
>+    for(; templIt != templDoNameVecs.end(); ++templIt){
>+        templDo = *templIt;

Put { on a new line after functions:

>+void WMLTemplateElement::setHasIntrEvent(bool has) {
>+    m_hasIntrEvent = has;
>+}
>+
>+bool WMLTemplateElement::hasIntrEvent() {
>+    return m_hasIntrEvent;
>+}

In general my advise is to code all sources to match webkit coding style. Until
then I think it makes sense to r- this, so that is what I do now.


-- 
Configure bugmail: https://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