From 986e6eec49b76d081374e5958401b98bcb1135c3 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Sun, 12 Oct 2008 13:44:23 -0400 Subject: [PATCH] Bug 448564. Better handling of sink context switches. r=mrbkap, sr=bzbarsky --- content/html/document/crashtests/448564.html | 7 ++ .../html/document/crashtests/crashtests.list | 1 + .../document/reftests/bug448564-1_ideal.html | 13 +++ .../reftests/bug448564-1_malformed.html | 19 ++++ .../reftests/bug448564-1_well-formed.html | 11 ++ .../reftests/bug448564-2_malformed.html | 11 ++ .../reftests/bug448564-2_well-formed.html | 14 +++ .../reftests/bug448564-3_malformed.html | 4 + .../reftests/bug448564-3_well-formed.html | 8 ++ .../html/document/reftests/bug448564-4a.html | 10 ++ .../html/document/reftests/bug448564-4b.html | 6 + .../reftests/bug448564-5_malformed.html | 16 +++ .../reftests/bug448564-5_well-formed.html | 17 +++ .../document/reftests/bug448564_forms.css | 2 + content/html/document/reftests/reftests.list | 10 ++ .../html/document/src/nsHTMLContentSink.cpp | 107 ++++++++---------- content/html/document/test/Makefile.in | 6 + content/html/document/test/bug448564-echo.sjs | 8 ++ .../document/test/bug448564-iframe-1.html | 16 +++ .../document/test/bug448564-iframe-2.html | 16 +++ .../document/test/bug448564-iframe-3.html | 16 +++ .../html/document/test/bug448564-submit.js | 4 + .../html/document/test/test_bug448564.html | 63 +++++++++++ layout/reftests/reftest.list | 3 + parser/htmlparser/src/CNavDTD.cpp | 30 ++++- parser/htmlparser/src/CNavDTD.h | 2 + 26 files changed, 362 insertions(+), 58 deletions(-) create mode 100644 content/html/document/crashtests/448564.html create mode 100644 content/html/document/reftests/bug448564-1_ideal.html create mode 100644 content/html/document/reftests/bug448564-1_malformed.html create mode 100644 content/html/document/reftests/bug448564-1_well-formed.html create mode 100644 content/html/document/reftests/bug448564-2_malformed.html create mode 100644 content/html/document/reftests/bug448564-2_well-formed.html create mode 100644 content/html/document/reftests/bug448564-3_malformed.html create mode 100644 content/html/document/reftests/bug448564-3_well-formed.html create mode 100644 content/html/document/reftests/bug448564-4a.html create mode 100644 content/html/document/reftests/bug448564-4b.html create mode 100644 content/html/document/reftests/bug448564-5_malformed.html create mode 100644 content/html/document/reftests/bug448564-5_well-formed.html create mode 100644 content/html/document/reftests/bug448564_forms.css create mode 100644 content/html/document/reftests/reftests.list create mode 100644 content/html/document/test/bug448564-echo.sjs create mode 100644 content/html/document/test/bug448564-iframe-1.html create mode 100644 content/html/document/test/bug448564-iframe-2.html create mode 100644 content/html/document/test/bug448564-iframe-3.html create mode 100644 content/html/document/test/bug448564-submit.js create mode 100644 content/html/document/test/test_bug448564.html diff --git a/content/html/document/crashtests/448564.html b/content/html/document/crashtests/448564.html new file mode 100644 index 00000000000..f31830afa13 --- /dev/null +++ b/content/html/document/crashtests/448564.html @@ -0,0 +1,7 @@ + +
a
+ + +
+ + diff --git a/content/html/document/crashtests/crashtests.list b/content/html/document/crashtests/crashtests.list index 5396b01fe22..b8b2554151b 100644 --- a/content/html/document/crashtests/crashtests.list +++ b/content/html/document/crashtests/crashtests.list @@ -1,3 +1,4 @@ load 388183-1.html load 395340-1.html load 407053.html +load 448564.html diff --git a/content/html/document/reftests/bug448564-1_ideal.html b/content/html/document/reftests/bug448564-1_ideal.html new file mode 100644 index 00000000000..e93c1771f66 --- /dev/null +++ b/content/html/document/reftests/bug448564-1_ideal.html @@ -0,0 +1,13 @@ + + + + + + + +
a +
b +
+ + diff --git a/content/html/document/reftests/bug448564-1_malformed.html b/content/html/document/reftests/bug448564-1_malformed.html new file mode 100644 index 00000000000..404517c70e5 --- /dev/null +++ b/content/html/document/reftests/bug448564-1_malformed.html @@ -0,0 +1,19 @@ + + + + + + + +
a +
b + + +
+ + + diff --git a/content/html/document/reftests/bug448564-1_well-formed.html b/content/html/document/reftests/bug448564-1_well-formed.html new file mode 100644 index 00000000000..46dbb8bdd08 --- /dev/null +++ b/content/html/document/reftests/bug448564-1_well-formed.html @@ -0,0 +1,11 @@ + + + + + + +
a +
b + + diff --git a/content/html/document/reftests/bug448564-2_malformed.html b/content/html/document/reftests/bug448564-2_malformed.html new file mode 100644 index 00000000000..ec676b5ae2a --- /dev/null +++ b/content/html/document/reftests/bug448564-2_malformed.html @@ -0,0 +1,11 @@ + + + +
a + + +
+
+ + + diff --git a/content/html/document/reftests/bug448564-2_well-formed.html b/content/html/document/reftests/bug448564-2_well-formed.html new file mode 100644 index 00000000000..f7657e58d74 --- /dev/null +++ b/content/html/document/reftests/bug448564-2_well-formed.html @@ -0,0 +1,14 @@ + + + +
a + +
+
+
+ + + + diff --git a/content/html/document/reftests/bug448564-3_malformed.html b/content/html/document/reftests/bug448564-3_malformed.html new file mode 100644 index 00000000000..01a52a06202 --- /dev/null +++ b/content/html/document/reftests/bug448564-3_malformed.html @@ -0,0 +1,4 @@ +
+ + +
head
diff --git a/content/html/document/reftests/bug448564-3_well-formed.html b/content/html/document/reftests/bug448564-3_well-formed.html new file mode 100644 index 00000000000..71f02c680ac --- /dev/null +++ b/content/html/document/reftests/bug448564-3_well-formed.html @@ -0,0 +1,8 @@ +
+ +
+ + +
head
diff --git a/content/html/document/reftests/bug448564-4a.html b/content/html/document/reftests/bug448564-4a.html new file mode 100644 index 00000000000..6fbaf85c2f2 --- /dev/null +++ b/content/html/document/reftests/bug448564-4a.html @@ -0,0 +1,10 @@ + + + + +
form contents
+ bold text +
+ + diff --git a/content/html/document/reftests/bug448564-4b.html b/content/html/document/reftests/bug448564-4b.html new file mode 100644 index 00000000000..f04d5fe48f7 --- /dev/null +++ b/content/html/document/reftests/bug448564-4b.html @@ -0,0 +1,6 @@ + + +
form contents
+ bold text + + diff --git a/content/html/document/reftests/bug448564-5_malformed.html b/content/html/document/reftests/bug448564-5_malformed.html new file mode 100644 index 00000000000..2a9391d76a3 --- /dev/null +++ b/content/html/document/reftests/bug448564-5_malformed.html @@ -0,0 +1,16 @@ + + + + + + +
+ + +
+ +
+ asdf + + diff --git a/content/html/document/reftests/bug448564-5_well-formed.html b/content/html/document/reftests/bug448564-5_well-formed.html new file mode 100644 index 00000000000..bdf65c1fd79 --- /dev/null +++ b/content/html/document/reftests/bug448564-5_well-formed.html @@ -0,0 +1,17 @@ + + + + + + +
+ +
+ +
+ asdf + + diff --git a/content/html/document/reftests/bug448564_forms.css b/content/html/document/reftests/bug448564_forms.css new file mode 100644 index 00000000000..b98788862c1 --- /dev/null +++ b/content/html/document/reftests/bug448564_forms.css @@ -0,0 +1,2 @@ +/* make nesting obvious */ +form { border: 1px solid black; } diff --git a/content/html/document/reftests/reftests.list b/content/html/document/reftests/reftests.list new file mode 100644 index 00000000000..3578976906e --- /dev/null +++ b/content/html/document/reftests/reftests.list @@ -0,0 +1,10 @@ +== bug448564-1_malformed.html bug448564-1_well-formed.html +== bug448564-1_malformed.html bug448564-1_ideal.html + +== bug448564-2_malformed.html bug448564-2_well-formed.html + +== bug448564-3_malformed.html bug448564-3_well-formed.html + +== bug448564-4a.html bug448564-4b.html + +== bug448564-5_malformed.html bug448564-5_well-formed.html diff --git a/content/html/document/src/nsHTMLContentSink.cpp b/content/html/document/src/nsHTMLContentSink.cpp index ca6c9dbdd05..7794f7fd7c6 100644 --- a/content/html/document/src/nsHTMLContentSink.cpp +++ b/content/html/document/src/nsHTMLContentSink.cpp @@ -373,6 +373,8 @@ public: nsGenericHTMLElement* mContent; PRUint32 mNumFlushed; PRInt32 mInsertionPoint; + + nsIContent *Add(nsIContent *child); }; Node* mStack; @@ -728,8 +730,10 @@ SinkContext::DidAddContent(nsIContent* aContent) } #endif - mSink->NotifyInsert(parent, aContent, - mStack[mStackPos - 1].mInsertionPoint - 1); + PRInt32 childIndex = mStack[mStackPos - 1].mInsertionPoint - 1; + NS_ASSERTION(parent->GetChildAt(childIndex) == aContent, + "Flushing the wrong child."); + mSink->NotifyInsert(parent, aContent, childIndex); mStack[mStackPos - 1].mNumFlushed = parent->GetChildCount(); } else if (mSink->IsTimeToNotify()) { SINK_TRACE(gSinkLogModuleInfo, SINK_TRACE_REFLOW, @@ -833,15 +837,7 @@ SinkContext::OpenContainer(const nsIParserNode& aNode) rv = mSink->AddAttributes(aNode, content); MaybeSetForm(content, nodeType, mSink); - nsGenericHTMLElement* parent = mStack[mStackPos - 2].mContent; - - if (mStack[mStackPos - 2].mInsertionPoint != -1) { - parent->InsertChildAt(content, - mStack[mStackPos - 2].mInsertionPoint++, - PR_FALSE); - } else { - parent->AppendChildTo(content, PR_FALSE); - } + mStack[mStackPos - 2].Add(content); NS_ENSURE_SUCCESS(rv, rv); @@ -900,6 +896,19 @@ SinkContext::HaveNotifiedForCurrentContent() const return PR_TRUE; } +nsIContent * +SinkContext::Node::Add(nsIContent *child) +{ + if (mInsertionPoint != -1) { + NS_ASSERTION(mNumFlushed == mContent->GetChildCount(), + "Inserting multiple children without flushing."); + mContent->InsertChildAt(child, mInsertionPoint++, PR_FALSE); + } else { + mContent->AppendChildTo(child, PR_FALSE); + } + return child; +} + nsresult SinkContext::CloseContainer(const nsHTMLTag aTag, PRBool aMalformed) { @@ -1151,19 +1160,8 @@ SinkContext::AddLeaf(nsGenericHTMLElement* aContent) if (mStackPos <= 0) { return NS_ERROR_FAILURE; } - - nsGenericHTMLElement* parent = mStack[mStackPos - 1].mContent; - - // If the parent has an insertion point, insert rather than append. - if (mStack[mStackPos - 1].mInsertionPoint != -1) { - parent->InsertChildAt(aContent, - mStack[mStackPos - 1].mInsertionPoint++, - PR_FALSE); - } else { - parent->AppendChildTo(aContent, PR_FALSE); - } - - DidAddContent(aContent); + + DidAddContent(mStack[mStackPos - 1].Add(aContent)); #ifdef DEBUG if (SINK_LOG_TEST(gSinkLogModuleInfo, SINK_ALWAYS_REFLOW)) { @@ -1199,26 +1197,17 @@ SinkContext::AddComment(const nsIParserNode& aNode) return NS_ERROR_FAILURE; } - nsGenericHTMLElement* parent; - if (!mSink->mBody && !mSink->mFrameset && mSink->mHead) { - // XXXbz but this will make DidAddContent use the wrong parent for - // the notification! That seems so bogus it's not even funny. - parent = mSink->mHead; - } else { - parent = mStack[mStackPos - 1].mContent; + { + Node &parentNode = mStack[mStackPos - 1]; + nsGenericHTMLElement *parent = parentNode.mContent; + if (!mSink->mBody && !mSink->mFrameset && mSink->mHead) + // XXXbz but this will make DidAddContent use the wrong parent for + // the notification! That seems so bogus it's not even funny. + parentNode.mContent = mSink->mHead; + DidAddContent(parentNode.Add(comment)); + parentNode.mContent = parent; } - // If the parent has an insertion point, insert rather than append. - if (mStack[mStackPos - 1].mInsertionPoint != -1) { - parent->InsertChildAt(comment, - mStack[mStackPos - 1].mInsertionPoint++, - PR_FALSE); - } else { - parent->AppendChildTo(comment, PR_FALSE); - } - - DidAddContent(comment); - #ifdef DEBUG if (SINK_LOG_TEST(gSinkLogModuleInfo, SINK_ALWAYS_REFLOW)) { mSink->ForceReflow(); @@ -1383,10 +1372,11 @@ SinkContext::FlushTags() #endif if ((mStack[stackPos].mInsertionPoint != -1) && (mStackPos > (stackPos + 1))) { + PRInt32 childIndex = mStack[stackPos].mInsertionPoint - 1; nsIContent* child = mStack[stackPos + 1].mContent; - mSink->NotifyInsert(content, - child, - mStack[stackPos].mInsertionPoint - 1); + NS_ASSERTION(content->GetChildAt(childIndex) == child, + "Flushing the wrong child."); + mSink->NotifyInsert(content, child, childIndex); } else { mSink->NotifyAppend(content, mStack[stackPos].mNumFlushed); } @@ -1487,18 +1477,9 @@ SinkContext::FlushText(PRBool* aDidFlush, PRBool aReleaseLast) return NS_ERROR_FAILURE; } - nsGenericHTMLElement* parent = mStack[mStackPos - 1].mContent; - if (mStack[mStackPos - 1].mInsertionPoint != -1) { - parent->InsertChildAt(mLastTextNode, - mStack[mStackPos - 1].mInsertionPoint++, - PR_FALSE); - } else { - parent->AppendChildTo(mLastTextNode, PR_FALSE); - } + DidAddContent(mStack[mStackPos - 1].Add(mLastTextNode)); didFlush = PR_TRUE; - - DidAddContent(mLastTextNode); } } @@ -1936,12 +1917,24 @@ HTMLContentSink::EndContext(PRInt32 aPosition) PRInt32 n = mContextStack.Count() - 1; SinkContext* sc = (SinkContext*) mContextStack.ElementAt(n); - NS_ASSERTION(sc->mStack[aPosition].mType == mCurrentContext->mStack[0].mType, + const SinkContext::Node &bottom = mCurrentContext->mStack[0]; + + NS_ASSERTION(sc->mStack[aPosition].mType == bottom.mType, "ending a wrong context"); mCurrentContext->FlushTextAndRelease(); + + NS_ASSERTION(bottom.mContent->GetChildCount() == bottom.mNumFlushed, + "Node at base of context stack not fully flushed."); - sc->mStack[aPosition].mNumFlushed = mCurrentContext->mStack[0].mNumFlushed; + // Flushing tags before the assertion on the previous line would + // undoubtedly prevent the assertion from failing, but it shouldn't + // be failing anyway, FlushTags or no. Flushing here is nevertheless + // a worthwhile precaution, since we lose some information (e.g., + // mInsertionPoints) when we end the current context. + mCurrentContext->FlushTags(); + + sc->mStack[aPosition].mNumFlushed = bottom.mNumFlushed; for (PRInt32 i = 0; imStackPos; i++) { NS_IF_RELEASE(mCurrentContext->mStack[i].mContent); diff --git a/content/html/document/test/Makefile.in b/content/html/document/test/Makefile.in index 9c316a7e5dd..9afcdea74bc 100644 --- a/content/html/document/test/Makefile.in +++ b/content/html/document/test/Makefile.in @@ -73,6 +73,12 @@ _TEST_FILES = test_bug1682.html \ test_form-parsing.html \ test_viewport.html \ test_documentAll.html \ + test_bug448564.html \ + bug448564-iframe-1.html \ + bug448564-iframe-2.html \ + bug448564-iframe-3.html \ + bug448564-echo.sjs \ + bug448564-submit.js \ $(NULL) libs:: $(_TEST_FILES) diff --git a/content/html/document/test/bug448564-echo.sjs b/content/html/document/test/bug448564-echo.sjs new file mode 100644 index 00000000000..0b6eea9d9dd --- /dev/null +++ b/content/html/document/test/bug448564-echo.sjs @@ -0,0 +1,8 @@ +function handleRequest(request, response) { + response.setHeader("Cache-Control", "no-cache", false); + response.setStatusLine(request.httpVersion, 200, "OK"); + + response.write(""); +} diff --git a/content/html/document/test/bug448564-iframe-1.html b/content/html/document/test/bug448564-iframe-1.html new file mode 100644 index 00000000000..4f3e79e5d27 --- /dev/null +++ b/content/html/document/test/bug448564-iframe-1.html @@ -0,0 +1,16 @@ + + + + + + + + + + +
+ + + + + diff --git a/content/html/document/test/bug448564-iframe-2.html b/content/html/document/test/bug448564-iframe-2.html new file mode 100644 index 00000000000..dba19b37e2d --- /dev/null +++ b/content/html/document/test/bug448564-iframe-2.html @@ -0,0 +1,16 @@ + + + +
+ + + + + +
+
+ + + + + diff --git a/content/html/document/test/bug448564-iframe-3.html b/content/html/document/test/bug448564-iframe-3.html new file mode 100644 index 00000000000..64288ebb15d --- /dev/null +++ b/content/html/document/test/bug448564-iframe-3.html @@ -0,0 +1,16 @@ + + + + +
+
+ + + + +
+ + + + + diff --git a/content/html/document/test/bug448564-submit.js b/content/html/document/test/bug448564-submit.js new file mode 100644 index 00000000000..34de44d4dd5 --- /dev/null +++ b/content/html/document/test/bug448564-submit.js @@ -0,0 +1,4 @@ +var inputs = document.getElementsByTagName("input"); +for (var input, i = 0; input = inputs[i]; ++i) + if ("submit" == input.type) + input.click(); diff --git a/content/html/document/test/test_bug448564.html b/content/html/document/test/test_bug448564.html new file mode 100644 index 00000000000..f565178ca9b --- /dev/null +++ b/content/html/document/test/test_bug448564.html @@ -0,0 +1,63 @@ + + + + + Test for Bug 448564 + + + + + + + +Mozilla Bug 448564 +

+ + + +

+ +
+
+
+ + diff --git a/layout/reftests/reftest.list b/layout/reftests/reftest.list index eb682d674f7..22834114d1c 100644 --- a/layout/reftests/reftest.list +++ b/layout/reftests/reftest.list @@ -125,3 +125,6 @@ include ../xul/base/src/grid/reftests/reftest.list # z-index/ include z-index/reftest.list + +# reftest(s) to verify content bugfixes +include ../../content/html/document/reftests/reftests.list diff --git a/parser/htmlparser/src/CNavDTD.cpp b/parser/htmlparser/src/CNavDTD.cpp index ae6c83a5511..3ad465b446a 100644 --- a/parser/htmlparser/src/CNavDTD.cpp +++ b/parser/htmlparser/src/CNavDTD.cpp @@ -1806,8 +1806,9 @@ CNavDTD::HandleSavedTokens(PRInt32 anIndex) PRInt32 attrCount; PRInt32 theTopIndex = anIndex + 1; PRInt32 theTagCount = mBodyContext->GetCount(); + PRBool formWasOnStack = mSink->IsFormOnStack(); - if (mSink->IsFormOnStack()) { + if (formWasOnStack) { // Do this to synchronize dtd stack and the sink stack. // Note: FORM is never on the dtd stack because its always // considered as a leaf. However, in the sink FORM can either @@ -1875,9 +1876,19 @@ CNavDTD::HandleSavedTokens(PRInt32 anIndex) result = HandleToken(theToken, mParser); } } + if (theTopIndex != mBodyContext->GetCount()) { + // CloseContainersTo does not close any forms we might have opened while + // handling saved tokens, because the parser does not track forms on its + // mBodyContext stack. CloseContainersTo(theTopIndex, mBodyContext->TagAt(theTopIndex), PR_TRUE); + } + + if (!formWasOnStack && mSink->IsFormOnStack()) { + // If a form has appeared on the sink context stack since the beginning of + // HandleSavedTokens, have the sink close it: + mSink->CloseContainer(eHTMLTag_form); } // Bad-contents were successfully processed. Now, itz time to get @@ -2707,6 +2718,19 @@ CNavDTD::OpenContainer(const nsCParserNode *aNode, return result; } +nsresult +CNavDTD::CloseResidualStyleTags(const eHTMLTags aTag, + PRBool aClosedByStartTag) +{ + const PRInt32 count = mBodyContext->GetCount(); + PRInt32 pos = count; + while (nsHTMLElement::IsResidualStyleTag(mBodyContext->TagAt(pos - 1))) + --pos; + if (pos < count) + return CloseContainersTo(pos, aTag, aClosedByStartTag); + return NS_OK; +} + /** * This method does two things: 1st, help construct * our own internal model of the content-stack; and @@ -2749,6 +2773,10 @@ CNavDTD::CloseContainer(const eHTMLTags aTag, PRBool aMalformed) if (mFlags & NS_DTD_FLAG_HAS_OPEN_FORM) { mFlags &= ~NS_DTD_FLAG_HAS_OPEN_FORM; done = PR_FALSE; + // If we neglect to close these tags, the sink will refuse to close the + // form because the form will not be on the top of the SinkContext stack. + // See HTMLContentSink::CloseForm. (XXX do this in other cases?) + CloseResidualStyleTags(eHTMLTag_form, PR_FALSE); } break; diff --git a/parser/htmlparser/src/CNavDTD.h b/parser/htmlparser/src/CNavDTD.h index d246c8fd3f5..ca1fd8246f1 100644 --- a/parser/htmlparser/src/CNavDTD.h +++ b/parser/htmlparser/src/CNavDTD.h @@ -310,6 +310,8 @@ private: nsresult CloseContainersTo(eHTMLTags aTag, PRBool aClosedByStartTag); nsresult CloseContainersTo(PRInt32 anIndex, eHTMLTags aTag, PRBool aClosedByStartTag); + nsresult CloseResidualStyleTags(const eHTMLTags aTag, + PRBool aClosedByStartTag); /** * Causes leaf to be added to sink at current vector pos.