[webkit-reviews] review denied: [Bug 28982] Basic MathML Rendering Support : [Attachment 39223] Update patch with support for the msqrt and mroot elements

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Sep 10 15:58:22 PDT 2009


Dave Hyatt <hyatt at apple.com> has denied Alex Milowski <alex at milowski.com>'s
request for review:
Bug 28982: Basic MathML Rendering Support
https://bugs.webkit.org/show_bug.cgi?id=28982

Attachment 39223: Update patch with support for the msqrt and mroot elements
https://bugs.webkit.org/attachment.cgi?id=39223&action=review

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
You've got a lot of style violations still, so let's get that taken care of.

Please fix the CSS file to be more readable:

selector {
   property/value pairs (one per line);
}

We no longer indent class declarations in header files (many of the files
violate this still).  They should just be flush left, e.g.,:

namespace WebCore {

class MathMLElement ....

}

Omit braces when you only have a single line for else statements or for stuff
inside a loop, e.g.,

else if (localName()=="mfrac") {
	  object = new (arena) RenderFraction(this);
      } 

^^^^ That is wrong.  Just drop the braces.

#endifs traditionally say what they are closing, e.g.,

#endif // MathMLElement_h

#endif // namespace WebCore

Make sure to have spaces in comparators and mathematical expressions, e.g.,:

if (localName()=="mfenced")

should be:

if (localName() == "mfenced")


More information about the webkit-reviews mailing list