417384 - "ASSERTION: Span level will be negative"

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Jesse Ruderman]

Attachment: testcase

Smaug, remember those assertions you added for in bug 409990? :)

###!!! ASSERTION: Span level will be negative!: 'mSpanLevel', file /Users/jruderman/trunk/mozilla/content/base/src/nsPlainTextSerializer.cpp, line 948

Cautiously filing as security-sensitive because the last time this kind of thing happened (bug 409990), it caused memory corruption.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

This isn't critical, since mSpanLevel isn't used for array indexing or anything.
But I'll look at this.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Attachment: proposed patch

Ok, this is bad after all. mInHead doesn't take account nested head elements.
So changing from bool to a counter.
But have to make nsPlainTextSerializer::OpenHead() no-op, though as far as I see
that isn't really used in serializer. It is just something coming from 
nsIHTMLContentSink and is used only by 
http://lxr.mozilla.org/seamonkey/source/parser/htmlparser/src/CNavDTD.cpp#3023
In nsHTMLContentSink OpenHead does open head context, but serializer
doesn't really have such concept.

Comment 3

[Jesse Ruderman]

Would it make sense to change the variable name from mInHead to mHeadNestingLevel now that it's not a boolean?

Comment 4

[Peter Van der Beken [:peterv]]

Comment on attachment 303269 [details] [diff] [review]
proposed patch

Let's name it mHeadLevel or something and we should add this to the testsuite.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Attachment: mHeadLevel

I don't know how to write a (mochitest or similar) testcase for this, because 
the interfaces nsPlainTextSerializer implements aren't scriptable.

Comment 6

[Jeff Walden [:Waldo]]

See attachment 286790 [details] [diff] [review]?

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Attachment: with test

Thanks Jeff!

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Attachment: with test

And the right file

Comment 9

[Jeff Walden [:Waldo]]

Credit where credit's due, I think -- philor, see comments 5-8.