Bug 10362

Created attachment 12145 [details] First attempt Note that this is just a start. I am not adding svg error reporting everywhere yet (so nothing is in place for reporting path syntax errors) since I'd like to know whether this is the right approach :) Cheers, Rob.

Comment on attachment 12145 [details] First attempt I don't like the idea of stopping parsing. Also fprintfs are a bad idea for the layout tests, we should just remove those. I think reporting errors is great. I don't think we should stop parsing however. Long term we could consider offering some sort of "debug mode" in Safari which would stop parsing, etc. But I still think that's probably a bad idea.

Created attachment 12193 [details] Improved patch This patch should be much better. It reports a lot of errors that occur during parsing and DOM changes. Cheers, Rob.

Comment on attachment 12193 [details] Improved patch we discussed a bunch of improvements over irc, including making lineNumber(), etc pure virtual, using contextElement() instead of passing document() and cleaning up the long ifs which repeatedly called the same error message.

Created attachment 12244 [details] Improved patch This should fix most of MacDome's issues. It doesnt seem worth it to make lineNumber() pure virtual, since three Tokenizer-derived classes in loader/ would need the same base implementation. It is hard to get the js line numbers. Maybe that needs to be done later. I also split out Tokenizer itself. That is not really required for this patch to work, but I thought it may make sense. Cheers, Rob.

Comment on attachment 12244 [details] Improved patch 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;

Created attachment 12278 [details] Improved patch Deciding to not touch Tokenizer too much since it is unrelated to the patch. This patch should address Sam's issues. Cheers, Rob.

Landed in r18652.