Closed Bug 55224 Opened 24 years ago Closed 23 years ago

switching from HTML Source mode to Normal causes dataloss

Categories

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

Product:
Core
Component:
DOM: Editor
Type:
defect

Tracking

()

Status:
VERIFIED DUPLICATE of bug 77902
Milestone:
mozilla1.0

People

(Reporter: Brade, Assigned: mozeditor)

References

Details

(Keywords: dataloss, regression, Whiteboard: [rtm-] relnote-user)

Attachments

(2 files)

sample file which exhibits dataloss in html source window
502 bytes, text/html
Details
Patch that fixes <map> and <textarea> data loss.
9.98 KB, patch
Details | Diff | Splinter Review
In Composer, if you go into HTML Source mode (View menu) and then switch back to Normal (or save), you can lose tags that were in your file! ouch!
Keywords: dataloss, regression, rtm
Priority: P3 → P1
Target Milestone: --- → M19
What tags get lost? How likely is the user to encounter this?
steps to reproduce: save/edit the attachment (10/4/2000 14:17) above view | html source view | normal at this point the <map> and <area> tags have been deleted and we've munged their file. There is no undo so there isn't a way to get this back unless the user realizes the tags are gone before they save (and then they don't save).
Severity: normal → critical
It's possible that this is another manufestation of bug 50965, which definitely causes dataloss on inserting html fragments (I had another bug where <script> tags were disappearing). Harish, does this sound like another clone of the same bug?
Status: NEW → ASSIGNED
Fix for bug 50965 would cover nodes with skipped-content ( SCRIPT,STYLE,TITLE,TEXTAREA,PLAINTEXT,XMP...) but not for AREA,MAP.. In otherwords, I don't think the problem, that Kathy described, is related to 50965!
I will take a look into it..though :)
Akkana, this one is critical, please hook up with Harish and see what can be done
Whiteboard: [rtm NEED INFO]
P2. Not a crasher, tags that are lost are not really common. akkana, will NOXIF fix this?
Priority: P1 → P2
This is similar to the other bug in that I'm passing the complete string in to In Kathy's attached example, here's the string I'm passing in to CreateContextualFragment(): <body> <img src="http://www.mozilla.org/images/mozilla-banner.gif" alt="alt" width="600" height="58" border="0" usemap="#back"><map name="back"><area shape="rect" coords="6,11,29,24" href="http://hotspot.com/" alt="alt text for hotspot"></map> </body> </html> and the fragment that results is: #document-fragment@0x8730520 refcount=2< Text@0x87329b8 refcount=2<\n> img@0x8732a44 src=http://www.mozilla.org/images/mozilla-banner.gif alt=alt width=600 height=58 border=0 usemap=#back refcount=2<> map@0x8732c80 name=back refcount=2< area@0x8732da8 shape=rect coords=6,11,29,24 href=http://hotspot.com/ alt=alt text for hotspot refcount=2<> > Text@0x8732f30 refcount=2<\n> > Looks like the information is still there in the fragment. So the editor must be doing something wrong when inserting the elements of the fragment. Cc'ing Joe, who recently wrote this code.
Joe sez probably the new DTD just needs to be taught about these tags, Passing to him. Joe, pass it back to me if you think it's probably something else.
Assignee: akkana → jfrancis
Status: ASSIGNED → NEW
oh the humanity
Status: NEW → ASSIGNED
can't see this making the rtm build, setting to rtm- and putting over in mozilla0.9
Whiteboard: [rtm NEED INFO] → [rtm-]
Target Milestone: M19 → mozilla0.9
:-( add relnoteRTM keyword do we know if there are any other tags or attributes that get eaten by switching modes?
Keywords: relnoteRTM
Whiteboard: [rtm-] → [rtm-] relnote-user
*** Bug 59880 has been marked as a duplicate of this bug. ***
I have a hard time entering <!DOCTYPE> tags in the HTML source, eg. <!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"> Whenever I save or go back to Normal, the DOCTYPE tag dissappears. I don't know if this is related to this bug, but it sounds like that. (Shouldn't DOCTYPE be supported in the first place?) I'm currently using Mozilla 0.7 (Mozilla/5.0 (X11; U; Linux 2.2.14 i686; en-US; 0.7) Gecko/20010105)
We have known problems supporting other doctype statements, and don't currently support changing the doctype. Charley knows more about that, I think (cc'ing).
We currently do not support changing the DOCTYPE in the editor. It is always: <!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"> If user could enter an arbitrary DOCTYPE, then we would have to support that DTD, which we currently don't do.
the doctype statement is not accessible in the editor, when the file is saved, a doctype staement s added if there isn't one in the document already
Keywords: mozilla0.8.1
Keywords: mozilla0.8
akkana, tony, did the stuff tony checked in a while ago that you and I reviewed fix this?
Kathy: you say there's "no undo". That shouldn't be true. The entire HTML source session should be undone after switching back to "Normal". Switching back does: Select all, delete, parse and insert new HTML source string. Also: When fix for bug 34458 is accepted, we will always insert the 4.01Transitional DOCTYPE for any new doc or doc that has no doctype. Maybe that will help?
We're still losing <textarea> tags. We should check all form elements to see what we are preserving, and what we aren't.
FYI, I have a fix for the disappearing <textarea> tags.
I have a patch that fixes the problem with the sample file (<map> disappearing), note that the fix is similar to the one I had to do for the disappearing <textarea> tags (bug #73659). We're going to have to do a sweep through all HTML4 tags paying special attention to container tags and see if switching from normal->viewsource->normal drops them ... if so, we will have to fix them in the same manner.
dont know if this matters or not, but i just looked at the patch and it looks good to me, i will apply it and test it when my build finishes. anthonyd
r = jfrancis
+ nsEditor::GetTagString(aNode,tag); + tag.ToLowerCase(); + if (tag.Equals(aTag)) Why not EqualsIgnoreCase() ? Thank you for refactoring! sr=sfraser
So I spent some time looking at the paser code that builds up the table used to decide the various properties of an HTML element, like whether or not it is a container, etc.: http://lxr.mozilla.org/seamonkey/source/htmlparser/src/nsElementTable.cpp Just from eyeballing the code the list of container exceptions we need to avoid dataloss might be quite large. I wonder if it's just better to: 1. Make a decision to remove empty containers based on the list of containers we know we want to strip out. (p, h1 .. h6, etc.) or 2. Just skip the whole removal of empty containers after inserting HTML, or at least inserting HTML as the result of switching back from view source mode. FYI, it seems that we don't do this removal pass during the initial load of the document. Of course I may be exaggerating the number of exceptions ... like I said, it was just from a quick glance. Comments?
Kin is right. For 0.9 I should switch over to use a specific list of elements that we will delete if empty; sometime down the line I should try to get rid of this code either altogether or at least make it used in fewer circumstances.
fix in hand
fix checked in
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
verified in 4/19 build.
Status: RESOLVED → VERIFIED
I think there is something wrong with the patch for map tag. http://www.npg.si.edu/exh/brady/gallery/gal01.html If you goto above URL and hit command+E to open in composer and then switch to HTML source, the last item within the map tag is driven out of the tag and inserted in the next line. 2001050408 trunk for Mac OS. Reopening.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
BTW, the link above is not exactly valid HTML if you look closely into the source (see my comment on bug 79078). Yet it is weird that the last item is moved to the next line. This is actually observable even when you select Show All Tags instead of HTML Source.
SPAM: mozilla 0.9 (and M1, and 0.8.1, etc.) has left the building. please update the target milestone so we can get a good idea of what's left for 0.9.1.
I need to make a "LoadHTML" method that does less validation than our InsertHML codepath. That will fix these issues once and for all.
Target Milestone: mozilla0.9 → mozilla1.0
dup of 77902 *** This bug has been marked as a duplicate of 77902 ***
Status: REOPENED → RESOLVED
Closed: 24 years ago → 23 years ago
Resolution: --- → DUPLICATE
verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: