[Webkit-unassigned] [Bug 28982] Basic MathML Rendering Support

[bugzilla-daemon at webkit.org]

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


Alexey Proskuryakov <ap at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever Confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW




--- Comment #1 from Alexey Proskuryakov <ap at webkit.org>  2009-09-07 10:12:05 PDT ---
Please use check-webkit-style on the patch - it's difficult to read at least
due to tab/space inconsistencies (you can open it in Safari to see what I'm
talking about, some lines look indented 4 spaces, and some 8 spaces).

+    } else {
+        m_separators = StringImpl::create(",");
+    }

We don't use braces around single line control clauses.

+    Length pad((int)(style()->fontSize()*0.1),Fixed);

Please use C++-style casts, add spaces around binary operators, and after
commas.

+    RenderInline *open = new (renderArena()) RenderInline(document());

Star goes to the left.

+    RefPtr<StringImpl>  m_separators;

One space before variable name.

+        // TODO: parse units

We use FIXME, not TODO.

+/*
+ *  RenderMathInlineContainer.cpp
+ *  WebCore
+ *
+ *  Created by Alex Milowski on 8/26/09.
+ *  Copyright 2009 Alex Milowski. All rights reserved.
+ *

This is different style than existing code, can you use just a single copyright
line?

+RenderMathSubSup::RenderMathSubSup(Node *subsup) :
RenderMathInlineContainer(subsup),m_scripts(0)

Initializers go on a separate line, indented four spaces:

+RenderMathSubSup::RenderMathSubSup(Node* subsup)
    : RenderMathInlineContainer(subsup)
    , m_scripts(0)
{
}

+void RenderMathInlineContainer::setStyle(PassRefPtr<RenderStyle> style) {

Opening brace goes on a separate line.

+    //newStyle->setBorderBottomColor(Color(255,0,0));
+    //newStyle->setBorderBottomWidth(1);
+    //newStyle->setBorderBottomStyle(SOLID);

We don't check in commented out code, unless there is a very good reason to
(such a reason needs to be given in a comment, of course).

I'm sorry that the feedback is so formal - someone more familiar with adding
new markup language support to WebKit will need to comment on the substance of
the patch. In the meanwhile, bringing the code in compliance with WebKit coding
standards will make the substantive review much easier.

Please mark patches that are up for review as "review?" when submitting, or
later by clicking on "Details" link to the right of the patch.

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