js: innerHTML removes 2 last symbols "]]"

RESOLVED FIXED in mozilla1.9alpha1

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
11 years ago
9 years ago

People

(Reporter: Constantin, Assigned: Robert Sayre)

Tracking

({fixed1.8.1.1})

Trunk
mozilla1.9alpha1
x86
Windows XP
fixed1.8.1.1
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(5 attachments, 3 obsolete attachments)

(Reporter)

Description

11 years ago
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

11 years ago
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.
I think this is fixed now...
Created attachment 224303 [details]
The code to reproduce the bug.
(Reporter)

Comment 4

11 years ago
Created attachment 224304 [details]
The code to reproduce the bug.

I changed resolution and document type to xhtml
Assignee: nobody → general
Component: General → DOM: Mozilla Extensions
Product: Firefox → Core
QA Contact: general → ian
Version: unspecified → Trunk
(Assignee)

Updated

11 years ago
Component: DOM: Mozilla Extensions → DOM: Core
(Assignee)

Comment 5

11 years ago
Created attachment 241071 [details]
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.
(Assignee)

Updated

11 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 6

11 years ago
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
Assignee: general → sayrer
(Assignee)

Comment 7

11 years ago
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.
(Assignee)

Comment 8

11 years ago
Looks like we're calling DidBuildContent with the closing ']' still available in nsParser::ParseFragment.
(Assignee)

Comment 9

11 years ago
Created attachment 241112 [details] [diff] [review]
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.
Attachment #241112 - Flags: review?(peterv)
(Assignee)

Comment 10

11 years ago
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.
Attachment #241112 - Flags: review?(peterv)
(Assignee)

Updated

11 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 11

11 years ago
Created attachment 241127 [details] [diff] [review]
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?
Attachment #241112 - Attachment is obsolete: true
Attachment #241127 - Flags: superreview?(peterv)
Attachment #241127 - Flags: review?(mrbkap)
(Assignee)

Comment 12

11 years ago
Created attachment 241129 [details] [diff] [review]
if there's a tagStack, add the first bit of end tag to the source buffer
Attachment #241127 - Attachment is obsolete: true
Attachment #241129 - Flags: superreview?(peterv)
Attachment #241129 - Flags: review?(mrbkap)
Attachment #241127 - Flags: superreview?(peterv)
Attachment #241127 - Flags: review?(mrbkap)
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
Attachment #241129 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 14

11 years ago
Created attachment 241343 [details] [diff] [review]
fix mrbkap's nits
Attachment #241129 - Attachment is obsolete: true
Attachment #241343 - Flags: superreview?(peterv)
Attachment #241129 - Flags: superreview?(peterv)
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
Attachment #241343 - Flags: superreview?(peterv) → superreview+
(Assignee)

Updated

11 years ago
Attachment #241343 - Flags: approval1.8.1.1?
(Assignee)

Comment 16

11 years ago
Checking in nsParser.cpp;
/cvsroot/mozilla/parser/htmlparser/src/nsParser.cpp,v  <--  nsParser.cpp
new revision: 3.394; previous revision: 3.393
done
(Assignee)

Updated

11 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Assignee)

Comment 17

11 years ago
(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

Updated

11 years ago
Summary: js: innerHTML removes 2 last simbols "]]" → js: innerHTML removes 2 last symbols "]]"
Comment on attachment 241343 [details] [diff] [review]
fix mrbkap's nits

approved for 1.8 branch, a=dveditz for drivers
Attachment #241343 - Flags: approval1.8.1.1? → approval1.8.1.1+
(Assignee)

Comment 19

11 years ago
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
Keywords: fixed1.8.1.1
(Assignee)

Updated

11 years ago
Target Milestone: --- → mozilla1.9alpha
(Assignee)

Comment 20

11 years ago
Test case here:

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

Comment 21

11 years ago
this is in mochitest now, so marking in-testsuite+
Flags: in-testsuite+
(Assignee)

Comment 22

10 years ago
Created attachment 255709 [details] [diff] [review]
move the unit test into parser/htmlparser/
Attachment #255709 - Flags: review?(mrbkap)
Comment on attachment 255709 [details] [diff] [review]
move the unit test into parser/htmlparser/

Neat!
Attachment #255709 - Flags: review?(mrbkap) → review+

Updated

9 years ago
Component: DOM: Core → DOM: Core & HTML
QA Contact: ian → general
You need to log in before you can comment on or make changes to this bug.