SVG needs to support SVGError events and some form of "error state"
The spec calls for implementing an "error state":
"A highly perceivable indication of error shall occur. For visual rendering situations, an example of an indication of error would be to render a translucent colored pattern such as a checkerboard on top of the area where the SVG content is rendered."
Most desktop browser authors would consider such error behavior complete lunacy, given the current expectation of web content authors for browsers to silently render any erroneous HTML page.
Regardless, we should have some way to display errors to the content author, even if we silently render erroneous content nicely for the end user. One possible solution would be to leverage an error console, such as is described by bug 10361.
Created attachment 12145 [details]
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 :)
Comment on attachment 12145 [details]
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]
This patch should be much better. It reports a lot of errors that occur during parsing and DOM changes.
Comment on attachment 12193 [details]
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]
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.
Comment on attachment 12244 [details]
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.
Put the brace on the same line as the class name.
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.
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.
As long as you are changing the return value, please fix the &.
+ bool parsePoints(const String &points) const;
Created attachment 12278 [details]
Deciding to not touch Tokenizer too much since it is unrelated to the patch. This patch should address
Landed in r18652.