Last Comment Bug 699365 - "ASSERTION: Unsupported parser command!" with CSS filter
: "ASSERTION: Unsupported parser command!" with CSS filter
Status: RESOLVED FIXED
: assertion, regression, testcase
Product: Core
Classification: Components
Component: HTML: Parser (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla11
Assigned To: Henri Sivonen (:hsivonen)
:
Mentors:
Depends on:
Blocks: randomstyles 343943 482921
  Show dependency treegraph
 
Reported: 2011-11-03 04:27 PDT by Jesse Ruderman
Modified: 2011-12-14 18:52 PST (History)
6 users (show)
hsivonen: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected


Attachments
testcase (74 bytes, text/html)
2011-11-03 04:27 PDT, Jesse Ruderman
no flags Details
stack trace (6.83 KB, text/plain)
2011-11-03 05:03 PDT, Jesse Ruderman
no flags Details
Check for extenal-resource in assertions (2.07 KB, patch)
2011-11-25 07:51 PST, Henri Sivonen (:hsivonen)
no flags Details | Diff | Review
Check for extenal-resource in assertions, with hg add (2.35 KB, patch)
2011-11-25 07:52 PST, Henri Sivonen (:hsivonen)
bugs: review+
gavin.sharp: approval‑mozilla‑aurora-
Details | Diff | Review

Description Jesse Ruderman 2011-11-03 04:27:54 PDT
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.
Comment 1 Jesse Ruderman 2011-11-03 05:03:40 PDT
Created attachment 571606 [details]
stack trace

aCommand is the string "external-resource".
Comment 2 Henri Sivonen (:hsivonen) 2011-11-03 06:35:31 PDT
(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.
Comment 3 Henri Sivonen (:hsivonen) 2011-11-03 06:37:24 PDT
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.
Comment 4 Henri Sivonen (:hsivonen) 2011-11-03 06:44:34 PDT
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
Comment 5 Daniel Veditz [:dveditz] 2011-11-09 17:09:29 PST
What does comment 4 mean: is it or not a security problem?
Comment 6 Johnny Stenback (:jst, jst@mozilla.com) 2011-11-16 16:59:39 PST
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...
Comment 7 Henri Sivonen (:hsivonen) 2011-11-17 11:33:56 PST
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.
Comment 8 Henri Sivonen (:hsivonen) 2011-11-25 07:51:09 PST
Created attachment 576934 [details] [diff] [review]
Check for extenal-resource in assertions
Comment 9 Henri Sivonen (:hsivonen) 2011-11-25 07:52:07 PST
Created attachment 576935 [details] [diff] [review]
Check for extenal-resource in assertions, with hg add
Comment 10 Henri Sivonen (:hsivonen) 2011-11-30 05:14:38 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/0354f6df10f7
Comment 11 Henri Sivonen (:hsivonen) 2011-11-30 07:03:02 PST
Backed out the test due to leak on Mac:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bcba9e0fcf4f
Comment 12 Marco Bonardo [::mak] 2011-12-01 04:01:48 PST
https://hg.mozilla.org/mozilla-central/rev/0354f6df10f7
https://hg.mozilla.org/mozilla-central/rev/bcba9e0fcf4f

marking fixed, though still lacks the test.
Comment 13 Henri Sivonen (:hsivonen) 2011-12-13 05:05:23 PST
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.
Comment 14 christian 2011-12-13 14:39:52 PST
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.
Comment 15 Henri Sivonen (:hsivonen) 2011-12-14 01:03:15 PST
Firefox 10 no longer affected due to bug 710175 landing.

Note You need to log in before you can comment on or make changes to this bug.