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.
191 bytes, text/html
Details
patch with fix
1.93 KB, patch
Details | Diff | Splinter Review
small change to the patch
1.18 KB, patch
Details | Diff | Splinter Review
back to the original diff because it works.
1.15 KB, 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. — 

    
    

    
  
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
        — 
          — Splinter Review
      

    


  

    
    

    
  
bug fix is attached.  got r= from akkana, need an sr= from harish.

anthonyd
Status: NEW → ASSIGNED
Whiteboard: FIX IN HAND

    
    

    
  


  

    
    

    
  
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: