[webkit-reviews] review denied: [Bug 42472] Implement MathML mfenced element : [Attachment 63372] Regenerated patch

[bugzilla-daemon at webkit.org]

Kenneth Rohde Christiansen <kenneth at webkit.org> has denied François Sausset
<sausset at gmail.com>'s request for review:
Bug 42472: Implement MathML mfenced element
https://bugs.webkit.org/show_bug.cgi?id=42472

Attachment 63372: Regenerated patch
https://bugs.webkit.org/attachment.cgi?id=63372&action=review

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
WebCore/mathml/MathMLInlineContainerElement.cpp:48
 +  : MathMLElement(tagName, document)
This seems wrong in accordance with your coding style guide

WebCore/mathml/RenderMathMLFenced.cpp:2
 +   * Copyright (C) 2009 Alex Milowski (alex at milowski.com). All rights
reserved.
Why doesn't this have your copyright?

WebCore/mathml/RenderMathMLFenced.cpp:43
 +	: RenderMathMLRow(fenced), 
, on the start of next line.

WebCore/mathml/RenderMathMLFenced.cpp:44
 +	m_open(0x28), 
0x28? Could we use an enum for these instead?

WebCore/mathml/RenderMathMLFenced.cpp:106
 +	    for (Node* position = child->node(); position; position =
position->previousSibling()) 
This part would need braces as the contents spans more than one physical line

WebCore/mathml/RenderMathMLFenced.cpp:113
 +		// use the last separator if we've run out of specified
separators
Start comments with capital and end with dot.

WebCore/mathml/RenderMathMLFenced.cpp:125
 +	// if we have a block, we'll wrap it in an inline-block
Same here

WebCore/mathml/RenderMathMLFenced.cpp:128
 +	    // block objects wrapper
here

WebCore/mathml/RenderMathMLFenced.cpp:145
 +  
why new line here?

WebCore/mathml/RenderMathMLFenced.cpp:151
 +  
and here?

WebCore/mathml/RenderMathMLFenced.cpp:162
 +	
and here?

WebCore/mathml/RenderMathMLFenced.h:40
 +  protected:
PErsonally I like a newline before protected: private: etc

WebCore/mathml/RenderMathMLOperator.cpp:43
 +  m_operator(0)
wrong change