Bug 3274 – document() not supported

• Component: XML
• Status: RESOLVED FIXED

[Dave Hyatt]

The document() capability of XSLT is not supported.  XSLT's loader API (the one used to load stylesheets) 
needs to be used in order to synchronously load and hand back an XML document.  This code could be 
similar to (or possibly reuse) the sync code for XMLHttpRequests.

[Anders Carlsson]

Created attachment 2555 [details]
Work in progress

Here's an initial stab at getting document() working. The things I'm concerned
with are:

1) The use of libxml: I see that other parts of WebCore use different parser
functions, for example XMLTokenizer::setTransformSource uses xmlReadMemory
instead, with a passed in QString. Should the QByteBuffer be encoded to a
QString (and what encoding should be used if so?) and then passed in.
Basically, what function should be used?

2) Error handling: If there's a parse error in an xml document included by
document(), the parse errors are written to stderr. I suppose the error sax
handlers need to be overridden in order to suppress those warnings.

3) Test cases: Not a big deal, but there are no real xsl test cases. Maciej
suggested that we add a fast/xsl subdir with xsl test cases. I haven't uploaded
any test cases here yet (I have a very basic one) since I wanted to get
feedback on the patch first.

[Anders Carlsson]

Comment on attachment 2555 [details]
Work in progress

I feel kinda stupid requesting a review on this patch since I know it's not
gonna be accepted, but I would like to get feedback on my previous comment.

[Darin Adler]

Comment on attachment 2555 [details]
Work in progress

Well, setting the review flag is indeed an effective way to get my attention!

Regarding your comments (2) and (3), sounds like you're on the right track. To
figure out what we should do in error cases, we first need to investigate what
other browsers do in such cases.

As far as issue (1) is concerned, the fact that other caller use xmlReadMemory
is due to the fact that we already have the data in memory, stored in a
QString. In the XMLTokenizer case we have already used our own decoding logic
for figuring out what character set to decode.

Whether libxml2's logic to determine what character set something is in is
acceptable or not, I don't really know. Since it doesn't get passed the
headers, I don't know how it could possibly handle that exactly right.

I think it's OK to start like this, and make a change only if we find a bug. If
we want to decode here in a way that matches the XMLTokenizer code path, it's
pretty simple:

Create a khtml::Decoder. Look at the headers and call setEncoding with the
encoding from the header, if any, passing EncodingFromHTTPHeader. See the logic
in KHTMLPart::write for other rules we might want to imitate (inheriting
encoding, user-specified encoding).

Call decode(), passing all the data. Call flush(), and append that QString to
the one you got from decode().

Then call xmlReadMemory the same way XMLTokenizer does.

I'm sure we could do a little refactoring to share code with KHTMLPart::write
too.

But it's probably not worth doing any of that until we make a test case and see
how the other browsers handle character sets in this case.

[Anders Carlsson]

Created attachment 2605 [details]
Adress comments

Looks like neither Gecko nor WinIE looks at the HTTP headers to see what the
encoding is, so I think we're fine using xmlReadMemory. I've also fixed the two
other issues.

[Darin Adler]

Comment on attachment 2605 [details]
Adress comments

I'd prefer that the layout test explain what it's testing and what the expected
result is.

There's some formatting strangeness here -- space after xmlSetGenericErrorFunc
but before the parenthesis.

Also there are break statements after return statements, which seems
unnecessary to me.

Should fix typo: "somwhere".

Otherwise, looks good.

[Anders Carlsson]

Created attachment 2628 [details]
Address more comments

[Darin Adler]

Comment on attachment 2628 [details]
Address more comments

r=me

[Joost de Valk (AlthA)]

Dave, could you verifiy this one? :)

[Adele Peterson]

*** Bug 4055 has been marked as a duplicate of this bug. ***

[Vicki Murley]

This bug is also in Radar as <rdar://4098268>