Closed
Bug 339350
Opened 18 years ago
Closed 18 years ago
js: innerHTML removes 2 last symbols "]]"
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha1
People
(Reporter: kichik.mainfo, Assigned: sayrer)
References
()
Details
(Keywords: fixed1.8.1.1)
Attachments
(5 files, 3 obsolete files)
562 bytes,
text/html
|
Details | |
521 bytes,
application/xhtml+xml
|
Details | |
969 bytes,
application/xhtml+xml
|
Details | |
3.08 KB,
patch
|
sicking
:
superreview+
dveditz
:
approval1.8.1.1+
|
Details | Diff | Splinter Review |
6.06 KB,
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
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•18 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.
Comment 2•18 years ago
|
||
I think this is fixed now...
Comment 3•18 years ago
|
||
Reporter | ||
Comment 4•18 years ago
|
||
I changed resolution and document type to xhtml
Updated•18 years ago
|
Assignee: nobody → general
Component: General → DOM: Mozilla Extensions
Product: Firefox → Core
QA Contact: general → ian
Version: unspecified → Trunk
Assignee | ||
Updated•18 years ago
|
Component: DOM: Mozilla Extensions → DOM: Core
Assignee | ||
Comment 5•18 years ago
|
||
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•18 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Comment 6•18 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•18 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•18 years ago
|
||
Looks like we're calling DidBuildContent with the closing ']' still available in nsParser::ParseFragment.
Assignee | ||
Comment 9•18 years ago
|
||
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•18 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•18 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 11•18 years ago
|
||
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•18 years ago
|
||
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 13•18 years ago
|
||
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•18 years ago
|
||
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•18 years ago
|
Attachment #241343 -
Flags: approval1.8.1.1?
Assignee | ||
Comment 16•18 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•18 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 17•18 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•18 years ago
|
Summary: js: innerHTML removes 2 last simbols "]]" → js: innerHTML removes 2 last symbols "]]"
Comment 18•18 years ago
|
||
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•18 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•18 years ago
|
Target Milestone: --- → mozilla1.9alpha
Assignee | ||
Comment 20•18 years ago
|
||
Test case here: http://intertwingly.net/blog/2006/10/03/Firefox-XHTML-innerHTML-quirk
Assignee | ||
Comment 21•18 years ago
|
||
this is in mochitest now, so marking in-testsuite+
Flags: in-testsuite+
Assignee | ||
Comment 22•18 years ago
|
||
Attachment #255709 -
Flags: review?(mrbkap)
Comment 23•18 years ago
|
||
Comment on attachment 255709 [details] [diff] [review] move the unit test into parser/htmlparser/ Neat!
Attachment #255709 -
Flags: review?(mrbkap) → review+
You need to log in
before you can comment on or make changes to this bug.
Description
•