Closed Bug 1647252 Opened 11 months ago Closed 1 month ago

remove use of nsISAXXMLReader in XMPP


(Chat Core :: XMPP, task, P2)


(thunderbird_esr78 wontfix, thunderbird88 wontfix)

89 Branch
Tracking Status
thunderbird_esr78 --- wontfix
thunderbird88 --- wontfix


(Reporter: mkmelin, Assigned: rnons)




(2 files)

+++ This bug was initially created as a clone of Bug #1514669 +++

Bug 1447707 removed nsISAXXMLReader from m-c, and forked to commm-central in bug 1518552. This bug is to remove the nsISAXXMLReader usage from XMPP. See also bug 1514666

Curious what the reasoning is behind this? Is there a reasonable replacement?

It's the last use of nsISAXXMLReader in the whole code base except CalDAV, which should be reasonably straight forward to use normal DOM parsing). It's c++, so not web compatible.

I don't know what the reasonable replacement is - someone will have to investigate. Either it's reasonable to use DOM parsing instead, or maybe there's some JavaScript SAX library available we could use.

Thanks! One somewhat annoying thing with XMPP is that you don't get a full document for every message, so you do need some kind of incremental parsing. Not sure if that's doable with DOM parsing or not though!

DOMParser does not support incremental parsing. XHR should, but it's up to you to remove the old nodes from the DOM in order to avoid the DOM growing without bounds.

It looks like could also provide a solution.

See Also: → 1514666
Assignee: nobody → remotenonsense

I tried and found DOMParser unsuitable for this. A few cases that doesn't work

  1. Tag is often not closed

    <?xml version='1.0'?><stream:stream ...><stream:features>...</stream:features>

  2. No xmlns:stream in stream:features. In a full document, xmlns can be inherited from the parent node, but if only one stanza is passed to DOMParser, it will error


  3. No single root node and redundant closing tag

    <presence .../><presence .../></presence>

I can work around some of the problems but I think it's unreliable.

I found a JS lib, but haven't tried it yet. What I see is to use it we need to copy 10+ files into c-c, should I give it a try?

Could be worth a shot.

Target Milestone: --- → 89 Branch

Pushed by
Use sax-js to replace nsISAXXMLReader in xmpp-xml.jsm. r=clokep

Pushed by
add sax-js license notice (ISC). rs=me

(In reply to Pulsebot from comment #10)

Pushed by
add sax-js license notice (ISC). rs=me

The license file was already added in comment 9 at comm/chat/protocols/xmpp/lib/sax/LICENSE

Pushed by
followup - remove duplicated LICENSE file. rs=me

Pushed by
Remove unused nsISAXXMLReader. r=mkmelin

Closed: 1 month ago
Resolution: --- → FIXED
Regressions: 1708695
You need to log in before you can comment on or make changes to this bug.