Closed Bug 75873 Opened 23 years ago Closed 22 years ago

HTML comments in framesets moved to the head

Categories

(Core :: DOM: HTML Parser, defect)

Product:
Core
Component:
DOM: HTML Parser
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
minor

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Future

People

(Reporter: wo, Assigned: bzbarsky)

Details

(Keywords: testcase, Whiteboard: need r=, sr=, a=)

Attachments

(4 files, 1 obsolete file)

testcase
295 bytes, text/html
Details
HTML file before editing in composer
341 bytes, text/html
Details
HTML file after editing in composer
409 bytes, text/html
Details
Patch updated to review comments
1.42 KB, patch
Details | Diff | Splinter Review 
Are HTML comments not allowed in frameset elements?

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Frameset//EN" 
   "http://www.w3.org/TR/html4/frameset.dtd">
<html>
<head><title></title></head>
<frameset rows="*,*">
<!-- 1st comment -->
    <frame src="about:blank">
<!-- 2nd comment -->
    <frame src="about:blank">
</frameset>
</html>

This is a valid document according to the w3c HTML validator, but Mozilla shifts 
all the comments out of the frameset and into the head. You can see this by 
calling
   javascript:alert(document.documentElement.innerHTML)
in the location field or by insepecting the DOM tree.
(Tested with NB 2001041004.)
Attached file testcase — 

    
    

    
  
I see this on linux (build from 2001-04-12) with document inspector
Keywords: testcase
OS: Windows 98 → All

    
    

    
  
Handing to parser/dtd guru...

html@03442BF0 refcount=3<
  head@034400A0 refcount=2<
    title@033F9DF0 refcount=2<
      Text@033F9D80 refcount=2<>
    >
    Comment@033F99B0 refcount=2<!-- 1st comment -->
    Comment@033F96B0 refcount=2<!-- 2nd comment -->
  >
  Text@033F9CB0 refcount=3<\n>
  frameset@033F9BC0 rows=*,* refcount=4<
    frame@033F9820 src=about:blank refcount=4<>
    frame@033FA5E0 src=about:blank refcount=4<>
  >
>
Assignee: pollmann → harishd
Status: UNCONFIRMED → NEW
Ever confirmed: true

    
    

    
  
This bug has been marked "future" because the original netscape engineer working 
on this is over-burdened. If you feel this is an error, that you or another
known resource will be working on this bug,or if it blocks your work in some way
-- please attach your concern to the bug for reconsideration. 
Status: NEW → ASSIGNED
Target Milestone: --- → Future

    
    

    
  
nsresult
SinkContext::AddComment(const nsIParserNode& aNode)
{
  .......
      nsIHTMLContent* parent;
      if ((nsnull == mSink->mBody) && (nsnull != mSink->mHead)) {
        parent = mSink->mHead;
      }
      else {
        parent = mStack[mStackPos - 1].mContent;
      }
....
}

This is where the bug is.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        simpleminded patch (obsolete)
        — 
          — Splinter Review
      

    


  

    
    

    
  
The patch fixes the behavior with the given testcase.  Should we be comparing to
nsnull though?  Or would something more like:

if (!mSink->mBody && !mSink->mFrameset && mSink->mHead) 

be better? 



    
  
Keywords: patch

    
  
Keywords: review
Whiteboard: need r=, sr=, a=

    
    

    
  
->Parser
Status: ASSIGNED → NEW
Component: HTMLFrames → Parser
QA Contact: amar → moied

    
    

    
  
I'm seeing a very similar symptom in a regular HTML document, not a frameset. 
It's a document with HTML comments for Apache server-side directives.  So the
movement of the comments ruins the document.  This is with Mozilla 1.2.1 on
Windows XP+SP1.

Attaching a before and after document.  Here's the sequence I followed.

Edit before.html document in notepad
Save a copy as after.html also
Open after.html in composer with right-click/edit in explorer
Click HTML Source tab
Type "la la la" after "PROJECT_HTML_HERE"
Click preview tab
Click HTML Source tab
The rearranged comments are visible on-screen
Save file.
The rearranged comments are seen in the file.


    
  

        Attachment #32043 -
        Flags: superreview?(heikki)

        Attachment #32043 -
        Flags: review?(harishd)

    
    

    
  
Comment on attachment 32043 [details] [diff] [review]
simpleminded patch

While you're there could you add a null check for mSink? 

r=harishd

        Attachment #32043 -
        Flags: review?(harishd) → review+

    
    

    
  
Comment on attachment 32043 [details] [diff] [review]
simpleminded patch

Use |if (foo && !bar)| style instead and you have sr=heikki.

        Attachment #32043 -
        Flags: superreview?(heikki)

        Attachment #32043 -
        Flags: superreview+

        Attachment #32043 -
        Flags: review?(harishd)

        Attachment #32043 -
        Flags: review+

    
  

        Attachment #32043 -
        Flags: review?(harishd) → review+

    
    

    
  
--> bz
Assignee: harishd → bzbarsky

    
    

    
  


  

        Attachment #32043 -
        Attachment is obsolete: true

    
    

    
  
fixed
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: