DOMParser RFE issues
Alex Vincent
edit XML documents via a textarea. I plan on using DOMParser to do
quick well-formedness checking. However, this presents a couple of
problems for me.
First, the expat parser loves to spew XML well-formedness errors
directly to the JS console. This is appropriate for a browser, but for
a XML editor it's a pain. The only way I find out the document isn't
well-formed is when DOMParser.parseFromString() returns a XML document
with a root element name of "parsererror".
Second, parsererror elements live in the null namespace. So if there's
a person who is nutty enough to edit a XML document with a root element
of parsererror, I have no way of knowing that it's a real document.
Third, when I was doing my research for this, I noticed
nsExpatDriver::HandleError() returns NS_ERROR_HTMLPARSER_STOPPARSING...
and no one checks the return value.
I'm wondering how to make expat (and my use of DOMParser) a little better.
Most of the time I just want a yea or nay on well-formedness, and if it
isn't well-formed, I want my application to catch the details of the
error - not have it logged to the console directly.
In addition, because I don't want to affect how mainline Mozilla code
uses these applications (the XML well-formedness error is useful as-is
to others), I'd wonder how to architecturally do it in a way that didn't
regress current behavior. This may mean that for the purpose I list
above, I may not want to use DOMParser - but instead create (and maybe
design?) a different component.
Before I file a bug on this, I'd like feedback on the Right Thing(s) To
Do. If the three issues above wouldn't be considered a valid RFE, I'd
like to know.
Alex Vincent
Martin Honnen
> Second, parsererror elements live in the null namespace. So if there's
> a person who is nutty enough to edit a XML document with a root element
> of parsererror, I have no way of knowing that it's a real document.
No, I am pretty sure that with Mozilla 1.0 and later that parsererror
root element is in the namespace
http://www.mozilla.org/newlayout/xml/parsererror.xml e.g. if you have
var xmlDocument = new DOMParser().parseFromString('<gods>',
'application/xml');
alert(
xmlDocument.documentElement.tagName + '\r\n' +
xmlDocument.documentElement.namespaceURI);
it shows
parsererror
http://www.mozilla.org/newlayout/xml/parsererror.xml
I have tested that now with Firefox 1.5.0.3 but I am pretty sure that
there is no difference in earlier releases.
--
Martin Honnen
http://JavaScript.FAQTs.com/
Boris Zbarsky
> First, the expat parser loves to spew XML well-formedness errors
> directly to the JS console.
Actually, it's the XML content sink that does this. We probably do want to have
a way to config this off for editors, yeah...
> Second, parsererror elements live in the null namespace. So if there's
> a person who is nutty enough to edit a XML document with a root element
> of parsererror, I have no way of knowing that it's a real document.
There's an existing bug for better error reporting from DOMParser.
> Third, when I was doing my research for this, I noticed
> nsExpatDriver::HandleError() returns NS_ERROR_HTMLPARSER_STOPPARSING...
> and no one checks the return value.
The method could probably be void. The one caller already returns the right
thing (NS_ERROR_HTMLPARSER_STOPPARSING).
> I'm wondering how to make expat (and my use of DOMParser) a little better.
None of this code is part of expat, fwiw.
> In addition, because I don't want to affect how mainline Mozilla code
> uses these applications (the XML well-formedness error is useful as-is
> to others), I'd wonder how to architecturally do it in a way that didn't
> regress current behavior.
Most simple would be a property on DOMParser that can be toggled and an API
extension to nsIContentSink, I would guess.
A different component might work too; you'd basically need to implement your own
XML sink then -- one that just ignores all the callbacks except the
error-reporting one. That would save you having to construct the DOM too, so
maybe it's the way to go.
-Boris
Robert Sayre
>
> A different component might work too; you'd basically need to implement
> your own XML sink then -- one that just ignores all the callbacks except
> the error-reporting one. That would save you having to construct the
> DOM too, so maybe it's the way to go.
I would use nsISAXXMLReader and set an errorHandler.
-Rob
ajvi...@gmail.com
> I would use nsISAXXMLReader and set an errorHandler.
:-( No dice. I still got an error message in the Error Console.
say...@gmail.com
Unfortunately, this message is coming from nsExpatDriver.cpp, from
https://bugzilla.mozilla.org/show_bug.cgi?id=297814
-Rob
Jonas Sicking
I would go with bzs suggestion and adding the capability to turn off
error-reporting from nsExpatDriver. This capability could be added to
both the SAX reader and to nsDOMParser.
I even wonder if the right thing to do is to default to not reporting
errors into the console and only enable it when loading things into a
docshell, if at all.
/ Jonas
Alex Vincent
> I would go with bzs suggestion and adding the capability to turn off
> error-reporting from nsExpatDriver. This capability could be added to
> both the SAX reader and to nsDOMParser.
I don't see an obvious way to do so.
> I even wonder if the right thing to do is to default to not reporting
> errors into the console and only enable it when loading things into a
> docshell, if at all.
I have a patch for your review in bug 342164, but you're suggesting here
my patch is wrong. :(
Jonas Sicking
My initial thought was that in the cases where we have other ways to
detect errors we should not log them to the error console. So for
example when we show the "yellow screen of error" there is not really a
point in logging to the error console as well. Similarly when we
dispatch an error event (such as for XMLHttpRequest) that should be
sufficient too.
Especially for scripted error-events, always reporting to the console
makes the error almost an "uncatchable exception", thus making the
error-event much less useful since the author has to avoid them rather
than rely on them.
However after reading the bug where the error logging was originally
added (bug 297814) I realized that in almost all cases people do not
catch error events. The analogy to exceptions doesn't really work since
we do log exceptions in the console if no-one catches them.
So I think the right solution is to default to logging errors, but
making it possible not to report them if the script handles them. I can
think of two ways of doing this:
1. Add a property on XMLHttpRequest and the sax reader that tells the
parser if errors should be reported or not. The default would be to
report them.
2. Make the default action of error-events be to log the error to the
console. This way if scripts handles the event and calls
.preventDefault() on them we will not log.
I like the second solution best, but it doesn't quite work for the
sax-reader since it doesn't have error-events. But we could do something
similar even for SAX.
/ Jonas
Alex Vincent
>I can think of two ways of doing this:
> console. This way if scripts handles the event and calls
> .preventDefault() on them we will not log.
>
> I like the second solution best, but it doesn't quite work for the
> sax-reader since it doesn't have error-events. But we could do something
> similar even for SAX.
That sounds almost exactly like what I did for my initial patch; my
patch didn't do reporting if there wasn't an error handler, but making
the change to report by default would be relatively trivial.
Or did you mean something else?
Robert Sayre
>
> 2. Make the default action of error-events be to log the error to the
> console. This way if scripts handles the event and calls
> .preventDefault() on them we will not log.
>
> I like the second solution best, but it doesn't quite work for the
> sax-reader since it doesn't have error-events. But we could do something
> similar even for SAX.
I don't see the use case for SAX console writing. SAX does provide its
own error event, via the ErrorHandler interface. I'd rather the
consuming application do the logging.
So far, none of the SAX use cases need/want console logging, and it
would be actively harmful in Thunderbird, where a whole sidebar full of
feeds can get refreshed at once.
-Rob
Jonas Sicking
I don't see your patch doing anything for events? Note that i'm talking
about DOM-Events.
/ Jonas
Jonas Sicking
I think the right thing to do for SAX is to use the error-handler if one
is provided, but report to the console if no error-handler was set. This
should prevent thunderbird from getting swamped, while making sure that
someone that doesn't consider errors will get warnings somewhere.
/ Jonas
Alex Vincent
>> That sounds almost exactly like what I did for my initial patch; my
>> patch didn't do reporting if there wasn't an error handler, but making
>> the change to report by default would be relatively trivial.
>
> I don't see your patch doing anything for events? Note that i'm talking
> about DOM-Events.
Ah, right, this has nothing to do with DOM events (and I'm a bit
confused as to where that enters the conversation, since you really
can't have an error event without a document - but I'll take enlightenment).
Alex
Alex Vincent
> I think the right thing to do for SAX is to use the error-handler if one
> is provided, but report to the console if no error-handler was set. This
> should prevent thunderbird from getting swamped, while making sure that
> someone that doesn't consider errors will get warnings somewhere.
>
> / Jonas
If that is the basis on which you will grant r+, I will submit a patch
to do so.
Robert, I'll need pointers to where in Thunderbird code (or elsewhere
SAXReader is used) to add an null-op error handler.
Alex
Robert Sayre
> Jonas Sicking wrote:
>> I think the right thing to do for SAX is to use the error-handler if
>> one is provided, but report to the console if no error-handler was
>> set.
That's a good idea.
>
> Robert, I'll need pointers to where in Thunderbird code (or elsewhere
> SAXReader is used) to add an null-op error handler.
It doesn't use it yet, but it will. No worries.
-Rob
Boris Zbarsky
> Ah, right, this has nothing to do with DOM events (and I'm a bit
> confused as to where that enters the conversation, since you really
> can't have an error event without a document
XMLHttpRequest has an onerror handler, etc.
-Boris