[webkit-reviews] review denied: [Bug 10362] SVG needs to support SVGError events and some form of "error state" : [Attachment 12244] Improved patch

[bugzilla-request-daemon at macosforge.org]

Sam Weinig <sam at webkit.org> has denied Sam Weinig <sam at webkit.org>'s request
for review:
Bug 10362: SVG needs to support SVGError events and some form of "error state"
http://bugs.webkit.org/show_bug.cgi?id=10362

Attachment 12244: Improved patch
http://bugs.webkit.org/attachment.cgi?id=12244&action=edit

------- Additional Comments from Sam Weinig <sam at webkit.org>
r- for a couple style issues

In Tokenizer.h (I do realize that this is just a copy and paste but we might as
well get it right):

There should not be a trailing underscore.

+#ifndef Tokenizer_h_
+#define Tokenizer_h_

Put the brace on the same line as the class name.

+class Tokenizer
+{

Indent the initialization list and put the braces on two lines.

+    Tokenizer(bool viewSourceMode = false) 
+    : m_parserStopped(false)
+    , m_inViewSourceMode(viewSourceMode)
+    {}

You should also indent the whole class declaration.

Add comments to closing brace and #endif.

+}
+
+#endif


In SVGDocumentExtensions.cpp:

The lines (seen in both reportWarning() and reportError())

+	 int lineNumber = 1;
+	 if (m_doc->tokenizer())
+	     lineNumber =  m_doc->tokenizer()->lineNumber();

could be clearer if written in one line, like:

    int lineNumber = m_doc->tokenizer() ? m_doc->tokenizer()->lineNumber() : 1;


(note that I got rid of the double space :) )


The changes in SVGFitToViewBox.h don't seem to have anything todo with the
patch.


In SVGParserUtilities.h:

As long as you are changing the return value, please fix the &.

+	 bool parsePoints(const String &points) const;