339350 - js: innerHTML removes 2 last symbols "]]"

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Constantin]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; ru; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; ru; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3

html-code:

<div id="someBlock" onclick="javascript:updateMe();">someText</div>

js-code:

function updateMe(){
    someBlock = document.getElementById("someBlock");
    someBlock.innerHTML = "[[someNewText]]";
}

result:

<div id="someBlock" onclick="javascript:updateMe();">[[someNewText</div> 

-- without "]]"

Reproducible: Always

Steps to Reproduce:
1.Create a page sush as in Details
2.Run in Firefox created page
3.Click on div "someText"
4.See result

Actual Results:  
[[someNewText

Expected Results:  
[[someNewText]]

May be that the problem is in the CDATA process

Comment 1

[Zook Valem]

A familiar pattern to the resolved fixed bug on the different javascript's object (function) somewhere but don't remember what it was.  That was almost a year ago to my knowledge.

Comment 2

[yspam88-bugzillamozilla]

I think this is fixed now...

Comment 3

[yspam88-bugzillamozilla]

Attachment: The code to reproduce the bug.

Comment 4

[Constantin]

Attachment: The code to reproduce the bug.

I changed resolution and document type to xhtml

Comment 5

[Robert Sayre]

Attachment: another test case

from: http://intertwingly.net/blog/2006/10/03/Firefox-XHTML-innerHTML-quirk

    * Displays properly in Opera 9.02 when served as application/xhtml+xml

    * Displays properly in IE when served as text/html

    * Displays properly in Firefox 1.5.0.7 when served as text/html

    * Omits trailing closing bracket in innerHTML on Firefox 1.5.0.7 when served as application/xhtml+xml

Adding more brackets to the tail end of the string results in the last two being removed.  Adding a space at the end makes the brackets show up.

Comment 6

[Robert Sayre]

Looks like we are buggy in nsXMLContentSink::HandleCharacterData or nsXMLContentSink::AddText. expat sends two character data events:

"foo [bar]"
 --------   <-- aLength is 8

"]</div></td></tr></table></body>..."
 -          <-- aLength is 1

Comment 7

[Robert Sayre]

ok, so the content stack is something like

>>>> tr->td->div->#document-fragment


the first character chunk is getting added to 

>>>> tr->td->div->#document-fragment

second character chunk is getting added to 

>>>> tr->td->div

and we return the nsIDOMDocumentFragment to InnerHTML, which doesn't contain the second chunk.

Comment 8

[Robert Sayre]

Looks like we're calling DidBuildContent with the closing ']' still available in nsParser::ParseFragment.

Comment 9

[Robert Sayre]

Attachment: add a piece of endtag to the fragment

I'm not proud of this, but it does fix the bug. Not sure we should mess with the tokenizer.

Comment 10

[Robert Sayre]

Comment on attachment 241112 [details] [diff] [review]
add a piece of endtag to the fragment

hmm, that is going to break XML with no context stack. hrmph.

Comment 11

[Robert Sayre]

Attachment: if there's a tagStack, add the first bit of end tag to the source buffer

Looks like mrbkap's patch for bug 263053 (remove <endnote>) regressed this. Where can I add a unit test?

Comment 12

[Robert Sayre]

Attachment: if there's a tagStack, add the first bit of end tag to the source buffer

Comment 13

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 241129 [details] [diff] [review]
if there's a tagStack, add the first bit of end tag to the source buffer

>+  } else {
>+

Nit: Remove this empty line.

>+        if (theIndex > 0)
>+          endContext.AppendLiteral("</");

Nit: Add braces to this if statement and the if/else below.

>+        nsAutoString thisTag( (PRUnichar*)aTagStack.ElementAt(theIndex) );
>+        PRInt32 endOfTag = thisTag.FindChar(PRUnichar(' '));  // was there an xmlns=?

Nits: Since you're here, could you remove those odd spaces before and after the parens? Also, wrap these lines at 80 chars.

Looks good otherwise. r=mrbkap

Comment 14

[Robert Sayre]

Attachment: fix mrbkap's nits

Comment 15

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 241343 [details] [diff] [review]
fix mrbkap's nits

>+        if (endOfTag == -1) {
>+          endContext.Append(thisTag);
>+        } else {
>+          endContext.Append(Substring(thisTag,0,endOfTag));

You can use StringHead(thisTag, endOfTag)

>+      result = Parse(endContext, (void*)&theContext, aMimeType,
>+                     PR_FALSE, PR_TRUE, aMode);

there should be no need for the cast to (void*)

sr=sicking with that

Comment 16

[Robert Sayre]

Checking in nsParser.cpp;
/cvsroot/mozilla/parser/htmlparser/src/nsParser.cpp,v  <--  nsParser.cpp
new revision: 3.394; previous revision: 3.393
done

Comment 17

[Robert Sayre]

(In reply to comment #11)
>
> Where can I add a unit test?

Nowhere, so I set up my own server :)

http://people.mozilla.com/~sayrer/Mozilla_DHTML/tests/

 
bug 357523

Comment 18

[Daniel Veditz [:dveditz]]

Comment on attachment 241343 [details] [diff] [review]
fix mrbkap's nits

approved for 1.8 branch, a=dveditz for drivers

Comment 19

[Robert Sayre]

Checking in nsParser.cpp;
/cvsroot/mozilla/parser/htmlparser/src/nsParser.cpp,v  <--  nsParser.cpp
new revision: 3.370.4.6; previous revision: 3.370.4.5
done

Comment 20

[Robert Sayre]

Test case here:

http://intertwingly.net/blog/2006/10/03/Firefox-XHTML-innerHTML-quirk

Comment 21

[Robert Sayre]

this is in mochitest now, so marking in-testsuite+

Comment 22

[Robert Sayre]

Attachment: move the unit test into parser/htmlparser/

Comment 23

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 255709 [details] [diff] [review]
move the unit test into parser/htmlparser/

Neat!