346444 - WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed: file nsExpatDriver.cpp, line 708

Status: NEW

(Core :: XML, defect)

Description

[Mats Palmgren (inactive)]

STEPS TO REPRODUCE
1. load testcase in a debug build

ACTUAL RESULT
A warning is printed on the console

EXPECTED RESULT
No warning on the console and perhaps even an XML Parsing Error in the
main content window? (it's blank currently).

Comment 1

[Mats Palmgren (inactive)]

Attachment: Testcase #1

Comment 2

[Mats Palmgren (inactive)]

Attachment: Testcase #2

Comment 3

[Mats Palmgren (inactive)]

Attachment: wip

Like so?
This makes it a parse error. Using "return XML_STATUS_OK" in the first block
fixes the warning and accepts the input silently.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Peter, can you check this out?

Comment 5

[Peter Van der Beken [:peterv]]

Comment on attachment 231246 [details] [diff] [review]
wip

>Index: parser/htmlparser/src/nsExpatDriver.cpp
>===================================================================

>+    return openEntityNames ? XML_STATUS_ERROR : XML_STATUS_OK;

Why make it conditional on openEntityNames?

Comment 6

[Mats Palmgren (inactive)]

Attachment: Testcase #3

(In reply to comment #5)
> Why make it conditional on openEntityNames?
> 

Because otherwise we would fail to parse DOCTYPEs, for example:
<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.0//EN"
                     "http://www.w3.org/TR/SVG/DTD/svg10.dtd">

Comment 7

[Mats Palmgren (inactive)]

  return !publicId ? XML_STATUS_ERROR : XML_STATUS_OK;

seem to work too. But it also gives a parse error for:
<!DOCTYPE svg SYSTEM "nonexistent">

(which the attached patch accepts).
I'm not sure if that's desirable or not.

Comment 8

[Peter Van der Beken [:peterv]]

(In reply to comment #7)
>   return !publicId ? XML_STATUS_ERROR : XML_STATUS_OK;
> 
> seem to work too. But it also gives a parse error for:
> <!DOCTYPE svg SYSTEM "nonexistent">
> 
> (which the attached patch accepts).
> I'm not sure if that's desirable or not.

So I'm not entirely sure, but I think we want this.

Comment 9

[Peter Van der Beken [:peterv]]

Comment on attachment 231246 [details] [diff] [review]
wip

See comment 8.

Comment 10

[Peter Van der Beken [:peterv]]

Actually, we should probably take into account whether the doc is standalone. Mats, let me know if you object to me taking this bug.

Comment 11

[Peter Van der Beken [:peterv]]

Note that if we follow the XML spec to the letter, both testcases shouldn't give an error. Only if they had standalone="yes" would there be an error (see the last paragraph in http://www.w3.org/TR/xml/#wf-entdeclared).

Comment 12

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b1pre) Gecko/20080927200111 Minefield/3.1b1pre] (home, debug default) (W2Ksp4)

'/toolkit/components/feeds/test' reftest reports
{
PASS | xml/rss09x/rss090.xml | Test was: "RSS 0.90 works" | feed.title.plainText() == "Mozilla Dot Org" && result.version == "rss090" |
*** loading ISO8601DateUtils
WARNING: NS_ENSURE_SUCCESS(rv, 1) failed with result 0x80004001: file .../parser/htmlparser/src/nsExpatDriver.cpp, line 735
PASS | xml/rss09x/rss091.xml | Test was: "RSS 0.91 works" | feed.title.plainText() == "Scripting News" && result.version == "rss091" |
WARNING: NS_ENSURE_SUCCESS(rv, 1) failed with result 0x80004001: file .../parser/htmlparser/src/nsExpatDriver.cpp, line 735
PASS | xml/rss09x/rss091_withNS.xml | Test was: "RSS 0.91 works" | feed.title.plainText() == "Scripting News" && result.version == "rss091" |
}