Closed Bug 67007 Opened 24 years ago Closed 24 years ago

Unknown tags dropped from composer source when switching views

Categories

(Core :: DOM: Editor, defect)

Product:
Core
Component:
DOM: Editor
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
major

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla0.8

People

(Reporter: bzbarsky, Assigned: anthonyd)

Details

(Whiteboard: FIX IN HAND)

Attachments

(4 files)

testcase for this.
(deleted), text/html
Details
patch with fix
(deleted), patch
Details | Diff | Splinter Review
small change to the patch
(deleted), patch
Details | Diff | Splinter Review
back to the original diff because it works.
(deleted), patch
Details | Diff | Splinter Review
This is a spinoff of bug 42781, but quite different. BUILD ID: Linux, 2001-01-29-08 STEPS TO REPRODUCE: 1) Open the file attached to this bug in composer. 2) Select "HTML Source" from the "View" menu 3) Note the presence of "<invalid></invalid>" in the source 4) Select "Show All Tags" from the "View" menu 5) Select "HTML Source" from the "View" menu ACTUAL RESULTS: "<invalid></invalid>" converted to "<br>" EXPECTED RESULTS: Source not affected by switching views. NOTES: This happens for all doctypes that I have tried, even ones that are not defined yet (HTML 6.0, for example). This is bad for Mozilla's forward-compatibility. "<img></img>" is not treated the same way -- it is replaced with "<img>" instead of being replaced with "<br>" This is a dataloss bug since there could simply be tags Mozilla is not recognizing in the source using some future doctype.
Attached file testcase for this. (deleted) —
Confirming on Mozilla trunk build 2001-01-19-04 Win2k. Changing OS to ALL. Note Composer will also convert a valid XML document to a default empty HTML document when switching views from source to normal edit mode as well.
OS: Linux → All
XML is not yet supported in editor, there needs to be an error message that inhibits the editor. Dropping unknown tags is the issue here, that should not be happening, Kin did an initial debug and will be updating the bug
ok, I created a file with: <invalid>I'm invalid text</invalid> displayed it in the browser, then selected File|Edit Page when the editor came up, I selected Debug|Dump Content Tree, and this is what was displayed: ============== Content Tree: ================ body@0FE7D890 refcount=11< invalid@10A239D0 _moz-userdefined= refcount=4< Text@10A23930 refcount=4<I'm invalid text> > > Then I toggled to HTML Source mode and entered another unknown element: <foo>this is a foo tag</foo> and toggled back to Normal mode, and selected Debug|Dump Content Tree again and this is what was displayed: ============== Content Tree: ================ body@0FE7D890 refcount=26< Text@10A60170 refcount=8<I'm invalid text\nthis is a foo tag\n> > I then selected Debug|Output HTML and this is what was displayed: Getting HTML <html><head></head><body>I'm invalid text this is a foo tag </body></html>
Interesting. So the parser isn't throwing away the tag on original parsing, but is throwing it away when we re-parse upon moving from source mode to normal mode. Charley, is it possible that we using a different DTD to reparse from source mode than the one we use when we initially start composer?
I haven't verified this yet, but I think it has something to do with the fact that we use nsHTMLEditor::InsertHTML(), to insert the code from the HTML Source TextArea, which eventually boils down to nsRange::CreateContextualFragment() which uses an nsHTMLFragmentContentSink(), which is different from the content sink used when initially loading the document in Composer.
reassign to cmanske
Assignee: beppe → cmanske
So what am I supposed to do with it! Harish: Kin is correct -- what can we do about getting nsHTMLFragmentContentSink to act like nsHTMLContentSink?
Verified that we are indeed calling InsertHTML with a string which includes the unknown tags, and they disappear after it's parsed back into the document. Note that the string passed to nsRange::CreateContextualFragment() does not include the doctype. So the fragment sink has no way to know what doctype we want. Two possible ways to fix this: 1. Easy fix to get us by for now: change the fragment sink so that it uses Transitional rather than Strict. At least, I assume that's easy -- Harish, is it? Better yet, since this is a Range method, and the range is already in a document, have it get the current doctype for its document, and use that. If we did that, then we might not need the more elaborate fix: 2. Eventually we probably want to change the CreateContextualFragment API so that it allows passing in a doctype string.
working with harish and akkana on this one. anthonyd
Assignee: cmanske → anthonyd
just a note in regards to how user agents should handle unknown elements per the W3C 4.01 HTML DTD. In Appendix B: Performance, Implementation and Design Notes, Section B.1: Notes on invalid documents This specification does not define how conforming user agents handle general error conditions, including how user agents behave when they encounter elements, attributes, attribute values, or entities not specified in this document. However, to facilitate experimentation and interoperability between implementations of various versions of HTML, we recommend the following behavior: 1.If a user agent encounters an element it does not recognize, it should try to render the element's content. 2.If a user agent encounters an attribute it does not recognize, it should ignore the entire attribute specification (i.e., the attribute and its value). 3.If a user agent encounters an attribute value it doesn't recognize, it should use the default attribute value. 4.If it encounters an undeclared entity, the entity should be treated as character data. We also recommend that user agents provide support for notifying the user of such errors. -------------------------------------- keeping these recommendations in mind, we should ignore/not process unknown elements and/or attributes, but should render the content. This fix will aide in reaching that goal.
Attached patch patch with fix (deleted) — — Splinter Review
bug fix is attached. got r= from akkana, need an sr= from harish. anthonyd
Status: NEW → ASSIGNED
Whiteboard: FIX IN HAND
Attached patch small change to the patch (deleted) — — Splinter Review
re-request for sr= from harishd anthonyd
Looks good to me. r=harishd
fixed and checked in. anthonyd
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Reopening. I had to back out the change to mozilla/layout/html/document/src/nsHTMLContentSink.cpp because it was causing blocker bug #67408.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
moving this one to moz0.8
Target Milestone: --- → mozilla0.8
ok, going back to the original fix, becaue everything works with it ;-). i will attach the new diff with the orginal fix, the dtd change was not back out, so it is not in the new diff. sorry for causing problems. i need new sr's and r's please. harish, please sr. anthonyd
Status: REOPENED → ASSIGNED
uh. r=harishd ;-)
fix checked in again ;-) anthonyd
Status: ASSIGNED → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → FIXED
verified fixed on linux build 2001-02-14-08
marking 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: