The default bug view has changed. See this FAQ.

"ASSERTION: Unsupported parser command!" with CSS filter

RESOLVED FIXED in mozilla11

Status

()

Core
HTML: Parser
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Jesse Ruderman, Assigned: hsivonen)

Tracking

(Blocks: 2 bugs, {assertion, regression, testcase})

Trunk
mozilla11
x86
Mac OS X
assertion, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox10 unaffected)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

6 years ago
Created attachment 571601 [details]
testcase

###!!! ASSERTION: Unsupported parser command!: '!nsCRT::strcmp(aCommand, "view")', file parser/html/nsHtml5Parser.cpp, line 720

This assertion is part of code added in:

changeset:   175a0afe3c43
user:        Henri Sivonen
date:        Fri Jul 30 13:15:38 2010 +0300
summary:     Bug 482921 part 1 - Implement HTML syntax highlighting using the new parser. r=Olli.Pettay.
(Reporter)

Comment 1

6 years ago
Created attachment 571606 [details]
stack trace

aCommand is the string "external-resource".
(Assignee)

Comment 2

6 years ago
(In reply to Jesse Ruderman from comment #1)
> aCommand is the string "external-resource".

Interesting. I wasn't aware such a parser command existed! It has been unsupported with the new HTML parser all along.

Has "external-resource" ever been properly tested with the HTML mode of the old parser? Or is it an XML thing that's leaking to HTML parsing accidentally?

We seriously need to change parser commands into an enum to make it obvious what the full set of possible commands is.
(Assignee)

Comment 3

6 years ago
Hiding this bug in case this reveals a security problem. I am not permitted to mark this a security bug, so I'm marking this as a different kind of confidential bug. Jesse, are you empowered to change this into a security bug.
Group: mozilla-corporation-confidential
(Assignee)

Comment 4

6 years ago
Looks like external-resource isn't a true *parser* command, since the old parser never checks it. It's checked in
https://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDocument.cpp#2353
Group: mozilla-corporation-confidential → core-security
What does comment 4 mean: is it or not a security problem?
Keywords: regression
I see no evidence of this being a security bug. Opening this up. Henri, can you look into what we should do about this in the parser? Asserting isn't right, but I'm not sure what is...
Assignee: nobody → hsivonen
Group: core-security
(Assignee)

Comment 7

5 years ago
So it looks like this parser command isn't really meant for the parser. It's meant as a magic value for nsDocument and the parser seems to be expected to fall back to a default.

Not quite nice to use a parser command like this, but it seems no actual harm is being done.
(Assignee)

Comment 8

5 years ago
Created attachment 576934 [details] [diff] [review]
Check for extenal-resource in assertions
Attachment #576934 - Flags: review?(bugs)
(Assignee)

Comment 9

5 years ago
Created attachment 576935 [details] [diff] [review]
Check for extenal-resource in assertions, with hg add
Attachment #576934 - Attachment is obsolete: true
Attachment #576934 - Flags: review?(bugs)
Attachment #576935 - Flags: review?(bugs)
Attachment #576935 - Flags: review?(bugs) → review+
(Assignee)

Comment 10

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/0354f6df10f7
Status: NEW → ASSIGNED
Flags: in-testsuite+
Target Milestone: --- → mozilla11
(Assignee)

Comment 11

5 years ago
Backed out the test due to leak on Mac:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bcba9e0fcf4f
Flags: in-testsuite+ → in-testsuite?
https://hg.mozilla.org/mozilla-central/rev/0354f6df10f7
https://hg.mozilla.org/mozilla-central/rev/bcba9e0fcf4f

marking fixed, though still lacks the test.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 13

5 years ago
Comment on attachment 576935 [details] [diff] [review]
Check for extenal-resource in assertions, with hg add

(This comment has been posted on all the bugs mentioned in this comment, except bug 710142, so that the release drivers see it regardless of the order in which they process approval requests.)

The new View Source implementation landed before Firefox 10 moved to Aurora. Afterwards, a bunch of regressions were identified. Many of the regression fixes didn't land before Firefox 10 moved to Aurora but they have now been fixed on trunk except for bug 710142.

To avoid shipping with regressions, we either need to land all the regression fixes on Aurora for Firefox 10 (followed by a fix for bug 710142 in Beta if it doesn't make it before Dec 20th) or switch back to the old View Source implementation on Aurora. The new View Source implementation provides much better diagnostics for Web developers than the old View Source implementation.

So I'd like to ask the release drivers to either approve bug 535530, bug 699356, bug 699365, bug 700034, bug 700361, bug 703965, bug 704667 and bug 705473 plus bug 695640, which is a non-regression tweak, or to approve bug 710175 for reverting to the old View Source implementation for Firefox 10.
Attachment #576935 - Flags: approval-mozilla-aurora?

Comment 14

5 years ago
Comment on attachment 576935 [details] [diff] [review]
Check for extenal-resource in assertions, with hg add

[triage comment]
We decided to back out the new view source parser (bug 710175) rather than take this fixup for Firefox 10.

Denying for Aurora.
Attachment #576935 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 15

5 years ago
Firefox 10 no longer affected due to bug 710175 landing.
status-firefox10: --- → unaffected
Attachment #576935 - Flags: approval-mozilla-aurora+ → approval-mozilla-aurora-
You need to log in before you can comment on or make changes to this bug.