Closed Bug 73605 Opened 25 years ago Closed 25 years ago

nsHTMLContentSerializer outputs <textarea> contents as attributes

Categories

(Core :: DOM: Editor, defect, P2)

Product:
Core
Component:
DOM: Editor
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.2

People

(Reporter: kinmoz, Assigned: anthonyd)

References

Details

(Keywords: dataloss, Whiteboard: [html][textarea], patch attached, need r= and sr=)

Attachments

(3 files, 1 obsolete file)

new patch with kin's changes
1.38 KB, patch
Details | Diff | Splinter Review
Reverse of previous patch, since bug 17003 is fixed now
1.35 KB, patch
sicking
: review+
jst
: superreview+
Details | Diff | Splinter Review
Same as previous patch, but this one applies in today's trunk build
1.98 KB, patch
Details | Diff | Splinter Review
I noticed that if I load the following content into Composer <html> <body> <form> <textarea>TextArea</textarea> </form> </body> </html> and switch to the ViewSource panel (or do a SaveAs), the resulting output from nsHTMLContentSerializer will look like this: <html> <body> <form> <textarea defaultvalue="TextArea" value="TextArea"></textarea> </form> </body> </html> This means that if you load the output into the browser or composer, the contents of the textarea will not be displayed and appears lost to the user! Vidur please reassign if you no longer work on the content serializer module.
Summary: nsHTMLContentSerializer outputs <textarea> contents out as attributes → nsHTMLContentSerializer outputs <textarea> contents as attributes
Anthony owns the serializers now.
Assignee: vidur → anthonyd
moving to mozilla1.0
Priority: -- → P2
Target Milestone: --- → mozilla1.0
*** Bug 82543 has been marked as a duplicate of this bug. ***
Depends on: 17003
moving this back into 9.2, this really needs to be fixed
Severity: normal → major
Keywords: correctness
Whiteboard: [html]
Target Milestone: mozilla1.0 → mozilla0.9.2
Whiteboard: [html] → [html][textarea]
Keywords: dataloss
Attached patch patch for this...YUCK (obsolete) — — Splinter Review
this bug sucks, and here is why: there is no underlying dom support for the content between the open element and the close element. so there isnt a way to correct this through the dom. so what has to be done is special case the serializer to check for textarea, and then do not set value and default value. then take value and shove it in between the open and close of the textarea. all other attributes are written out correctly. this means that if someone DOES set value or default value for a textarea, it will be stripped. c'est le vie. anthonyd
Status: NEW → ASSIGNED
Whiteboard: [html][textarea] → [html][textarea], patch attached, need r= and sr=
Harish is working on a fix for the broken textarea handling in mozilla, once that's done there shouldn't need to be any special code in the serializers...
Hi Anthony, got a mid-air collision with jst, but here's what I was going to say: I have found one HTML bug that would be concerned by this : bug 17003. (which appears to be the bug harish is working on) Two other bugs are connected, even though they concern more XUL. However it is possible that they suffer from the same problem, since xul probably uses the html textarea. bug 53673 bug 42646 If this patch addresses none of the issues above please ignore this comment. But I would be interested to get your feedback, whether they have anything to do with this. Thanks, Fabian.
So when is harish's fix supposed to land? Before 0.9.3 closes? If not, do we want this to go in as a workaround? In case this has to go in as a temporary workaround, here are some comments on the patch: The code added in the following should be moved after the BR case so that the comment and the BR code can be kept together. @@ -258,6 +258,11 @@ // Filter out special case of <br type="_moz"> or <br _moz*>, // used by the editor. Bug 16988. Yuck. // + if ( (aTagName == nsHTMLAtoms::textarea) && + ((attrName.get() == nsHTMLAtoms::value) || + (attrName.get() == nsHTMLAtoms::defaultvalue)) ){ + continue; + } if ((aTagName == nsHTMLAtoms::br) && (attrName.get() == nsHTMLAtoms::type) && (valueStr.EqualsWithConversion(kMozStr, PR_FALSE, sizeof(kMozStr)-1))) { In this part of the diff, the code added should be put at the end of the method it's in, or at least after the LineBreakAfterOpen() call just in case people want breaks automatically inserted after the <textarea> tag. @@ -342,6 +347,12 @@ AppendToString(kGreaterThan, aStr); + if (name.get() == nsHTMLAtoms::textarea) + { + nsAutoString valueStr; + content->GetAttribute(kNameSpaceID_HTML, nsHTMLAtoms::value, valueStr); + AppendToString(valueStr, aStr); + } if (LineBreakAfterOpen(name, hasDirtyAttr)) { AppendToString(mLineBreak, aStr); mColPos = 0; I'd also like to request that you add a comment that precedes the code you are adding (in both places) that says something to the effect: // XXX: This special cased textarea code should be // removed when bug #17003 is fixed. So that this workaround is easily flagged, and can be removed when the bug is fixed. If you can convince jst and harishd that this should go in, you can have my r=kin@netscape.com with those changes.
With kin's suggested changes made, sr=jst
If you're serializing the document might it make more sense to use the defaultValue property than the value property? (I guess it depends how the serializer is being used... if it's always used from an editor instantiated from parsing the page then it would probably make sense as is, but if it's used in other cases then the defaultValue property would make more sense and would be what you would get once bug 17003 is fixed.) But anyway, a=dbaron for trunk checkin (on behalf of drivers)
I have made kins changes. I will checkin. r=kin sr=jst a=dbaron anthonyd
its in. anthonyd
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
verified in 6/21 build.
Status: RESOLVED → VERIFIED
Attachment #126854 - Flags: superreview?(jst)
Attachment #126854 - Flags: review?(bugmail)
Comment on attachment 126854 [details] [diff] [review] Reverse of previous patch, since bug 17003 is fixed now sr=jst. Should this bug be reopened now?
Attachment #126854 - Flags: superreview?(jst) → superreview+
This patch is the same as the previous patch, but this one applies again in today's trunk build.
Martijn, have you tested that the behavior doesn't change with that patch?
Thanks, patch checked in now.
Oops, I didn't test this either, figured it was all good. I'll test once I have a fresh seamonkey build with this change. I'll back out the backout if there are problems :)
Ok, tested (using a 1.7 branch tree) and this change doesn't break textarea serialzation.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: