611042 - providing request.response.xml is nontrivial in Jetpack Processes

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect)

Description

[Atul Varma [:atul]]

Because native XML parsing and DOM APIs aren't available in Jetpack processes, we need to either figure out a way to provide request.response.xml, or cut it from the request module api.

(I did try using E4X, but it doesn't seem to provide a DOM API to XML.)

The only way I can think of providing request.response.xml that I can think of is to parse the XML in JavaScript. There are a few libraries lying around the web that do this, but it'd add some amount of overhead to the size of XPIs... Not that XPIs aren't already too large, weighing-in at around 180k for a simple "hello world" extension.

We should decide whether we want to remove the 'xml' property soon though, so that developers have time to adapt.

Comment 1

[Atul Varma [:atul]]

The 'rexml' library looks like it might work:

  http://www.levelthreesolutions.com/jsxml/

It's 6.8k uncompressed and probably isn't hard to CommonJS-ify, assuming that hasn't already been done by the node.js community or somesuch.

Comment 2

[Myk Melez [:myk] [@mykmelez]]

Hmm, indeed.  The only question is the license, as it is currently licensed only under the LGPL.  I'm not an expert on these issues, but I think we would probably need it to be licensed under either the tri-license or a BSD-style license.

But perhaps the owners of that code would be amenable to doing so.  In any case, it's not much worth worrying about until a technical analysis determines that it's a suitable and preferable library for providing this API.

Comment 3

[Atul Varma [:atul]]

The only problem with jsxml is that it doesn't provide a W3C DOM API to the document, which is what most people would expect.

The "XML for <SCRIPT>" library does contain a W3C DOM parser, but it requires two files, a SAX parser and a W3C DOM API, that weigh in at a combined ~80K compressed. Ugh.

I think the easiest solution for now is to not provide request.response.xml, but direct users to background pages and/or the available XML libraries if they really need to process XML in Jetpack-based addons.

Comment 4

[Atul Varma [:atul]]

The docs for the "XML for <SCRIPT>" library's W3C DOM parser are here, by the way:

  http://xmljs.sourceforge.net/website/documentation-w3cdom.html

Comment 5

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #3)
> The only problem with jsxml is that it doesn't provide a W3C DOM API to the
> document, which is what most people would expect.

More important than what they expect is what they need.  The W3C DOM API is pretty unwieldy, even if familiar, so if we can satisfy their use cases with a simpler one, we should consider doing so.


> I think the easiest solution for now is to not provide request.response.xml,
> but direct users to background pages and/or the available XML libraries if they
> really need to process XML in Jetpack-based addons.

Ok, let's do that and revisit as appropriate.

Comment 6

[Atul Varma [:atul]]

I guess my main concern about providing a W3C DOM is just that it also makes it easier to give it to other libraries that provide a much easier interface to DOM manipulation, like jQuery.

Anyways, sounds good, we will just cut it out for now.

BTW, I just realized that yet another option is to proxy all DOM calls over to the chrome process. It would be terribly inefficient, but could be accomplished fairly readily with JS Proxies, I think. Particularly given that XHR response DOMs are only being accessed by the Jetpack Process and there aren't any kind of event-based mechanisms in play, it ought to work.

Comment 7

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #6)
> BTW, I just realized that yet another option is to proxy all DOM calls over to
> the chrome process. It would be terribly inefficient, but could be accomplished
> fairly readily with JS Proxies, I think. Particularly given that XHR response
> DOMs are only being accessed by the Jetpack Process and there aren't any kind
> of event-based mechanisms in play, it ought to work.

Hmm, interesting idea, let's consider it if we determine at some point that it would be useful to reintroduce this functionality.

Comment 8

[Myk Melez [:myk] [@mykmelez]]

The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.

To filter bugmail related to this change, filter on the word "looptid".

Comment 9

[Atul Varma [:atul]]

Forged a pull request with changesets that remove response.xml:

https://github.com/mozilla/addon-sdk/pull/34

Comment 10

[Myk Melez [:myk] [@mykmelez]]

Reviewed and pulled: https://github.com/mozilla/addon-sdk/commit/9f202a3003cddace040bc695ab7137d4a31051ec.

Comment 11

[John Nagle]

It's amazing how fast this was done, and with how little discussion. Mozilla drops support for XML, and in 30 hours it's gone.  This deserves more publicity. 
At "https://developer.mozilla.org/en/XML_in_Mozilla", documentation says "Mozilla has extensive support for XML." Now that the Jetpack crowd has dropped XML support, that needs to be updated to reflect the policy change.

Comment 12

[Myk Melez [:myk] [@mykmelez]]

Mozilla generally has not dropped "support for XML", nor has the Jetpack project specifically, so there is no "policy change."  We simply removed a property from an API that we can't provide across process boundaries.

There are still plenty of ways for you to access and manipulate XML via Jetpack APIs, and friendly folks are happy to help you figure out how to do what you want to do in our support forums <https://forums.mozilla.org/addons/viewforum.php?f=27>. That's the place to go for assistance developing your XML-using addon.

Comment 13

[Bryan Clark (DevTools PM) [:clarkbw]]

(In reply to Atul Varma [:atul] from comment #0)
> Because native XML parsing and DOM APIs aren't available in Jetpack
> processes, we need to either figure out a way to provide
> request.response.xml, or cut it from the request module api.

I know this is resolved but I was wondering if I were to revert the pull request change up to the current code standard which has changed from the original.  Would that be accepted?

I agree there's no good XML parsing libraries in JavaScript and doubtful there will be any soon but implementers (such as myself) can still use DOM level queries to access the XML as it comes across as a document.

Currently I'm using getElementsByTagName to get a NodeList and then looping through that node for the various Nodes / NodeAttributes that I need.  

Example:

var list = responseXML.getElementsByTagName("Url");
for (var i = 0; i < list.length; i++) {
  var node = list.item(i);
  var type = node.getAttribute("type");
}

Given a somewhat trivial XML file it's not difficult to parse with those DOM level functions available.  Though the documentation does leave much to be desired. :)

So my proposal would be that I send a pull request for adding the xml attribute (with proper documentation):  (apologies for the ugly non-github formatting here)

  get xml() response(this).request.responseXML,

This would allow the same XML document access to the requests made as if they were XHR type requests.

(see current code: https://github.com/clarkbw/searchspot/blob/master/lib/search-engines-collector.js#L138)

Let me know if you'd consider taking a pull request for this and I'll try to put it all together.  Thanks for taking the time!

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I think we should let this bug rest in peace. Feel free to open a new bug about getting this property restored, though!

Comment 15

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Yeah, probably the simplest thing to do is: new bug, with a pull request.