Closed Bug 460437 Opened 16 years ago Closed 16 years ago

innerHtml doesn't update DOM when HTML markup goes from Invalid to Valid

Categories

(Core :: DOM: HTML Parser, defect, P2)

Product:
Core
Component:
DOM: HTML Parser
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.9.1b3

People

(Reporter: cray99, Assigned: bent.mozilla)

References

(
URL
)

Details

(4 keywords)

Attachments

(4 files, 1 obsolete file)

innerHtml; loss of DOM focus/relevance on updated elements
569 bytes, application/xhtml+xml
Details
mochitest.diff
2.09 KB, patch
Details | Diff | Splinter Review
Patch, v1
4.13 KB, patch
mrbkap
: review+
mrbkap
: superreview+
Details | Diff | Splinter Review
Patch for 1.9.0 branch
4.68 KB, patch
samuel.sidler+old
: approval1.9.0.11+
Details | Diff | Splinter Review
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.3) Gecko/2008092510 Ubuntu/8.04 (hardy) Firefox/3.0.3 Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.3) Gecko/2008092510 Ubuntu/8.04 (hardy) Firefox/3.0.3 In this bug demo, I have set up a textarea whose input feeds into an html element to keep it updated via JS innerHtml. Typing markup in the textarea works fine and the html element updates properly after the update button is clicked; however, if one inadvertently types incorrect markup and then clicks to update, the element disappears (properly so). BUT when one goes back and corrects the markup, the element never gains focus (relevance?) again. Also, if a page refresh is done via the browser, the original textarea data is not updated, one has to clear their data (cache? cookie?) and completely close the page and reopen it in order to get back to the original page and its origianl data. Reproducible: Always Steps to Reproduce: 1.type some invalid html markup into the textarea 2.update the info by clicking on the button 3.correct the html markup, try to update again. Actual Results: Nothing happens. The element begin updated by the innerHtml function is lost forever, never regaining focus or relevancy. Expected Results: The software should have rendered the corrected html markup after the corrections were made. I have an open source forum and mini-CMS in Xforms which relies on xforms-output via mediatype application/xhtml+xml depends heavily on this function working properly. My whole project (of two + years going) is at a stand still. Any help would be appreciated.
Please provide a site or a small HTML example that demonstrates the problem. You can upload files to the bug report using the "Add an attachment" link. Thanks.
sorry, forgot the code of my test case <html xmlns="http://www.w3.org/1999/xhtml"> <head> <title>Template</title> <script type="text/javascript"> function changeText2() { var userInput = document.getElementById('textArea').value; document.getElementById('boldStuff2').innerHTML = userInput; } </script> </head> <body> <textarea id="textArea" rows="10" cols="30"> By Dr. Suess </textarea> <p><b id='boldStuff2'>Cat in the hat</b> </p> <input type='button' onclick='changeText2()' value='Change Text'/> </body> </html>
sorry for the belated upload of the test case. Also, make sure you check out the animated gif of this case as well that was already posted. Thanks all.
I can reproduce the bug in Firefox 3.0.3, but not in 2.0.0.17, on Linux.
Status: UNCONFIRMED → NEW
Component: General → HTML: Parser
Ever confirmed: true
Keywords: regression, testcase
OS: Linux → All
Product: Firefox → Core
QA Contact: general → parser
Hardware: PC → All
Attached patch wip (obsolete) — — Splinter Review
This fixes it, but setting "mSink = nsnull" in nsParser::Initialize() effectively makes nsContentUtils::CreateContextualFragment() never reuse the sink. I can't find a corresponding "reset" method for the sink though so I don't see how it can be reused...
Is this a regression from bug 386769?
Blocks: 386769
Flags: blocking1.9.1?
Regression window: 2008-02-08-04 -- 2008-02-09-04. Yes, the checkin for bug 386769 is in that window.
As far as triage goes, we just need to either back out the performance optimization or fix it to produce correct output. We should definitely block on one or the other, imo.
Mats, if you won't be able to work on this please say so. We should attempt to fix this for 1.9.1. Ben, this seems related to a discussion I remember hearing about the cached parser not resetting its state in some cases. Is there another bug on this same problem, or was that about this bug?
Assignee: nobody → mats.palmgren
Flags: blocking1.9.1? → blocking1.9.1+
That was this bug, if it was the discussion I heard.
Over to Ben to get some traction here...
Assignee: mats.palmgren → bent.mozilla
Priority: -- → P2
Target Milestone: --- → mozilla1.9.1
Attached patch Patch, v1 — — Splinter Review
Let's try this. We only blow away the sink if there was an error.
Attachment #343589 - Attachment is obsolete: true
Attachment #351611 - Flags: superreview?(jst)
Attachment #351611 - Flags: review?(mrbkap)
Attachment #351611 - Flags: superreview?(jst)
Attachment #351611 - Flags: superreview+
Attachment #351611 - Flags: review?(mrbkap)
Attachment #351611 - Flags: review+
Comment on attachment 351611 [details] [diff] [review] Patch, v1 >diff --git a/parser/htmlparser/src/nsParser.cpp b/parser/htmlparser/src/nsParser.cpp >+ >+ if (killSink) { >+ mSink = nsnull; >+ } >+ Nit: Kill this extra newline. r+sr=mrbkap with that.
Summary: innerHtml doesn't update DOM when Html markup goes from Invalid to Valid → innerHtml doesn't update DOM when HTML markup goes from Invalid to Valid
Pushed changeset 3125e01b2164 to mozilla-central.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Pushed changeset 777f69dd5772 to mozilla-1.9.1.
Keywords: fixed1.9.1
Flags: in-testsuite+
Target Milestone: mozilla1.9.1 → mozilla1.9.1b3
Version: unspecified → Trunk
Depends on: 468538
==== JS Code ===== function xx(){ throw "Error"; } var a = "content before error"; try{ a=xx(); }catch(e){ } alert(a); =================== If we execute above code the value of variable "a" at the end is "content before error" So why the existing content of innerHTML get blanked when we try to set value to an invalid markup? See bug 474376 for test case
Because in the innerHTML case, we throw after already having cleared out the old childNodes (per my comment in bug 474376), i.e. just before the end of the set, whereas in your example you throw before the setter is invoked.
verified FIXED on builds: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090407 Shiretoko/3.5b4pre ID:20090407031108 and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090406 Minefield/3.6a1pre ID:20090406035346
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
So... we shipped 1.9.0 with this bug, didn't we? :( We should probably fix this on that branch too.
Flags: blocking1.9.0.10?
Yeah, seconded -- can we get a branch patch whipped up once 3.5b4 is sorted?
Flags: wanted1.9.0.x+
Flags: blocking1.9.0.10?
Flags: blocking1.9.0.10+
Code-freeze for 1.9.0.10 is April 22, could we get a patch by then or will we need to bump this to the next release?
Attachment #373875 - Flags: approval1.9.0.10?
Comment on attachment 373875 [details] [diff] [review] Patch for 1.9.0 branch Looks like a pretty clean port. Approved for 1.9.0.10. a=ss
Attachment #373875 - Flags: approval1.9.0.10? → approval1.9.0.10+
Fixed on cvs for 1.9.0.10.
Keywords: fixed1.9.0.10
Does this mean it should be fixed in FF 3.0.10? If so, the bug is not fixed. Or do I need to wait for another updated release of FF 3.0.* ? (Tested on Linux/Ubuntu with Ubuntu's Firefox and Mozilla's Firefox)
Unfortunately, what was Firefox 3.0.10 got bumped back to 3.0.11 to make way for a stability release. This bug will be fixed in the next release.
That's good news. I'm glad a decision was made to include this fix in the 3.0.* branch instead of having to wait for the 3.5.* branch. Thanks for all your efforts everyone.
Verified for 1.9.0.11 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.11pre) Gecko/2009051504 GranParadiso/3.0.11pre.
Keywords: fixed1.9.0.11 → verified1.9.0.11
Still broken with 3.0.11 on Linux. Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.11) Gecko/2009060308 Ubuntu/9.04 (jaunty) Firefox/3.0.11.
Sorry. Things are fine now. Mistake on my side. :)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: