DOMParser RFE issues

classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

DOMParser RFE issues

Alex Vincent
For my Verbosio project, at least initially, I'll be allowing people to
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
_______________________________________________
dev-tech-xml mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-xml
Reply | Threaded
Open this post in threaded view
|

Re: DOMParser RFE issues

Martin Honnen-3
Alex Vincent wrote:


> 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/
_______________________________________________
dev-tech-xml mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-xml
Reply | Threaded
Open this post in threaded view
|

Re: DOMParser RFE issues

Boris Zbarsky
In reply to this post by Alex Vincent
Alex Vincent wrote:
> 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
_______________________________________________
dev-tech-xml mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-xml
Reply | Threaded
Open this post in threaded view
|

Re: DOMParser RFE issues

Rob Sayre-2
Boris Zbarsky wrote:
>
> 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
_______________________________________________
dev-tech-xml mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-xml
Reply | Threaded
Open this post in threaded view
|

Re: DOMParser RFE issues

Alex Vincent
Robert Sayre wrote:
> I would use nsISAXXMLReader and set an errorHandler.

:-(  No dice.  I still got an error message in the Error Console.

_______________________________________________
dev-tech-xml mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-xml
Reply | Threaded
Open this post in threaded view
|

Re: DOMParser RFE issues

Rob Sayre-2
[hidden email] wrote:
> Robert Sayre wrote:
> > I would use nsISAXXMLReader and set an errorHandler.
>
> :-(  No dice.  I still got an error message in the Error Console.

Unfortunately, this message is coming from nsExpatDriver.cpp, from

https://bugzilla.mozilla.org/show_bug.cgi?id=297814

-Rob

_______________________________________________
dev-tech-xml mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-xml
Reply | Threaded
Open this post in threaded view
|

Re: DOMParser RFE issues

Jonas Sicking
[hidden email] wrote:
> [hidden email] wrote:
>> Robert Sayre wrote:
>>> I would use nsISAXXMLReader and set an errorHandler.
>> :-(  No dice.  I still got an error message in the Error Console.
>
> Unfortunately, this message is coming from nsExpatDriver.cpp, from
>
> https://bugzilla.mozilla.org/show_bug.cgi?id=297814

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
_______________________________________________
dev-tech-xml mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-xml
Reply | Threaded
Open this post in threaded view
|

Re: DOMParser RFE issues

Alex Vincent
Jonas Sicking wrote:
> 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.  :(
_______________________________________________
dev-tech-xml mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-xml
Reply | Threaded
Open this post in threaded view
|

Re: DOMParser RFE issues

Jonas Sicking
In reply to this post by Alex Vincent
I did some thinking, discussing and reading on this.

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
_______________________________________________
dev-tech-xml mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-xml
Reply | Threaded
Open this post in threaded view
|

Re: DOMParser RFE issues

Alex Vincent
Jonas Sicking wrote:
>I can think of two ways of doing this:
> 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.

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?
_______________________________________________
dev-tech-xml mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-xml
Reply | Threaded
Open this post in threaded view
|

Re: DOMParser RFE issues

Rob Sayre-2
In reply to this post by Jonas Sicking
Jonas Sicking wrote:
>
> 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
_______________________________________________
dev-tech-xml mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-xml
Reply | Threaded
Open this post in threaded view
|

Re: DOMParser RFE issues

Jonas Sicking
In reply to this post by Alex Vincent
Alex Vincent wrote:

> Jonas Sicking wrote:
>> I can think of two ways of doing this:
>> 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.
>
> 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.

/ Jonas
_______________________________________________
dev-tech-xml mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-xml
Reply | Threaded
Open this post in threaded view
|

Re: DOMParser RFE issues

Jonas Sicking
In reply to this post by Rob Sayre-2
Robert Sayre wrote:

> Jonas Sicking wrote:
>>
>> 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.

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
_______________________________________________
dev-tech-xml mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-xml
Reply | Threaded
Open this post in threaded view
|

Re: DOMParser RFE issues

Alex Vincent
In reply to this post by Jonas Sicking
Jonas Sicking wrote:
>> 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
_______________________________________________
dev-tech-xml mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-xml
Reply | Threaded
Open this post in threaded view
|

Re: DOMParser RFE issues

Alex Vincent
In reply to this post by Jonas Sicking
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. 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
_______________________________________________
dev-tech-xml mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-xml
Reply | Threaded
Open this post in threaded view
|

Re: DOMParser RFE issues

Rob Sayre-2
Alex Vincent wrote:
> 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
_______________________________________________
dev-tech-xml mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-xml
Reply | Threaded
Open this post in threaded view
|

Re: DOMParser RFE issues

Boris Zbarsky
In reply to this post by Alex Vincent
Alex Vincent wrote:
> 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
_______________________________________________
dev-tech-xml mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-xml