nsSAXXMLReader::HandleStartDTD calls nsISAXLexicalHandler::StartDTD with reversed systemID, publicID

RESOLVED FIXED in mozilla19

Status

()

Core
XML
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: WeirdAl, Assigned: WeirdAl)

Tracking

Trunk
mozilla19
x86_64
All
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
IDL for nsISAXLexicalHandler:
void startDTD(in AString name, in AString publicId, in AString systemId);

C++ call on startDTD:
  if (mLexicalHandler) {
    return mLexicalHandler->StartDTD(nsDependentString(aName),
                                     nsDependentString(aSystemId),
                                     nsDependentString(aPublicId));
  }

This makes SAX parsing... less than ideal.  Patch with testcase coming.
(Assignee)

Comment 1

6 years ago
Created attachment 680332 [details] [diff] [review]
patch without testcase

I thought about altering the IDL, but the SAX specification says the IDL is correct:
http://www.saxproject.org/apidoc/org/xml/sax/ext/LexicalHandler.html

Also, due to the limitations of XPCShell testing, I wasn't able to write a simple unit-test for this, either:  apparently I have to be able to load a DTD, and there aren't any handy in XPCShell land.  I could write a chrome mochitest, but that'll take a little longer.
Attachment #680332 - Flags: review?(bugs)

Comment 2

6 years ago
Comment on attachment 680332 [details] [diff] [review]
patch without testcase

I wonder why expat and SAX have the parameters in different order.


chrome test would be nice.
Attachment #680332 - Flags: review?(bugs) → review+
(Assignee)

Updated

6 years ago
Attachment #680332 - Flags: checkin?
Keywords: checkin-needed
Attachment #680332 - Flags: checkin?
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa2f835b67a5

Yes, test please! :)
Flags: in-testsuite?
Keywords: checkin-needed

Comment 4

6 years ago
https://hg.mozilla.org/mozilla-central/rev/aa2f835b67a5
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
You need to log in before you can comment on or make changes to this bug.