Closed Bug 159615 Opened 22 years ago Closed 21 years ago

Composer adds blank line between SCRIPT & STYLE tags when opening files

Categories

(Core :: DOM: Serializers, defect)

Product:
Core
Component:
DOM: Serializers
Platform:
x86
Windows XP
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: lucxaw, Assigned: harishd)

References

Details

Attachments

(4 files, 2 obsolete files)

test case
342 bytes, text/html
Details
Additional test case showing more problems switching between SOURCE & NORMAL views
1.25 KB, text/html
Details
testcase for both STYLE and SCRIPT bugs
1.01 KB, text/html
Details
patch v3.0
1.77 KB, patch
glazou
: review+
peterv
: superreview+
Details | Diff | Splinter Review 
I am seeing the problem in Bug 46227: “switching between ‘normal’ and ‘html 
source’ adds spaces” except it is occurring when Composer opens a document (and 
not when switching between “Normal” and “HTML source”). Extra blank lines are 
added to HTML sources between <head> and </head>. If you modify and save the 
file, then open it again, more blank lines are added.

Build ID: 2002072504
Attached file test case — 

    
    

    
  
Test Case attached.

Open “test.html” in composer. Switch to the Source view and notice how blank 
lines are added between <style> and <script> tags.

Modify the document by typing in some characters. Save, close, and reopen. 
Switch to Source view and you’ll notice more blank line are inserted.

    
  
Status: UNCONFIRMED → NEW
Ever confirmed: true

    
    

    
  
-->Myself
Assignee: harishd → t_mutreja

    
  
Status: NEW → ASSIGNED

    
    

    
  

      
      
      
      

        Attached patch
          
          
        PatchV1.0 (obsolete)
        — 
          — Splinter Review
      

    


  
Akanna, would you please see if this patch is causing any regression. I tried
all the test case I could. 
Also I need to check if it has some positive effect for bug# 114204.

    
    

    
  
Tanu: are these return characters really in the dom?  We should never have \r in
the dom -- that's an error.  So if that's what's causing the extra newlines, it
might be better to find out where the \r characters are coming from and fix that.

It might still be good to put your patch in the tree, but I'd be happier
tracking down why it's needed first.


    
    

    
  


  
Here is an additional test case that shows some more problems. It may or may
not be the same bug.

This occurs every time you switch from HTML SOURCE to NORMAL view after making
any edit.  This applies when "Retain Original Source Formatting" is selected in
Composer Preferences.

It has existed since at least August 2002, but I was waiting to see if fix to
Bug #145196 fixed it, which it did not.  (Bug #145196 fixed "Reformat HTML
source" mode)  

Note the test case file contains no blank lines or indentation, but both are
added when it is opened/edited in Composer.

    
    

    
  
From the original description, this sounds like a dupe of bug 145196.
Two separate issues were causing the newlines appearing in <script>/<style>
elements and the newlines in <head>.
I was just about to mark this as a duplicate of bug 145196, but then I just
realized it has a patch attached which fixes the bug 176861 which I just filed a
moment ago.

I filed bug 176866 for the case when pretty printing is off.


    
    

    
  
*** Bug 176861 has been marked as a duplicate of this bug. ***

    
    

    
  
Akkana, Yes, it seems that return characters are coming from the DOM. Also it's 
true only for tags inside HEAD. Dump Content shows this clearly. I'm looking 
into it and will try to update you asap.

    
    

    
  
In comment #9, Tanu states:
> Also it's true only for tags inside HEAD. Dump Content shows this clearly.

Tanu, Please check the test case in my Comment #6 above.  You'll find that many
closing tags in the body are also affected, including but not limited to <br>,
</p>, </address>, </h1>...</h6>.  Also, both <table> and </table>.  Should I
file this as a separate bug? 

    
    

    
  

      
      
      
      

        Attached patch
          
          
        PatchV2.0 (obsolete)
        — 
          — Splinter Review
      

    


  

    
    

    
  
CCing Harish for comments.

Jim:
>Should I file this as a separate bug? 
Here I tried to restrict "\r" in SCRIPT & STYLE tags to enter the DOM. So Jim, 
it would be a good idea to file a new bug for the testcase because those new 
lines are coming due to some other bug.

    
    

    
  
I already filed bug 176866 for that.


    
    

    
  
*** Bug 177122 has been marked as a duplicate of this bug. ***

    
    

    
  
Changing summary...
The testcase and patches here are specifically dealing with SCRIPT and STYLE 
tags. If there are any other similar problems, it would be better to open new 
bugs for them.
Summary: Composer adds blank line to HTML source when opening files → Composer adds blank line between SCRIPT & STYLE tags when opening files

    
    

    
  
*** Bug 177122 has been marked as a duplicate of this bug. ***

    
    

    
  


  
This testcase should be used for since the summary(and description) of this bug

has changed.
If you need more info about the SCRIPT bug, please read a complete detailed
description in bug #177122

    
    

    
  
Comment on attachment 104366 [details] [diff] [review]
PatchV2.0

What's the performance impact of this patch?

Is it possible to contain this patch to editor mode only?

    
    

    
  
Harish, I understand the concern here but if the specification says that DOM 
new lines must not include CR, then probably this would be the better(may be 
the only one) place. Otherwise I know where to deal with this in Serializer but 
no idea about other places.
Akkana/Harish would you please provide me some more pointers if I should think 
in a different manner.


    
    

    
  
Regarding patch v2.0:
The correct way to to have a new line in Win32 is to use: CR/LF
In Unix, the correct way is: LF

I read your comments in the patch and wonder if you are considering
the case when Composer is running on a Win32 platform.

    
    

    
  
The correct way to have a newline in the DOM is LF.  The serializers can convert
from DOM newlines to platform newlines, but when it's in the DOM it should do as
the DOM, otherwise lots of code that traverses the DOM can get confused.

    
    

    
  
Comment #21
ok, now I understand.

Another cuestion, should we change the summary or open a new bug for the
"Navigator->View->Page Source" issue?
Because this bug also happend in Navigator when you open the 
"Page Source" window.

Aditional info:
This bug happend *only* when composer opens the file. 
Not when it saves the file.
To verify this, try:
1. create a new composer window
2. insert the style/script tags(from the testcase)
3. save the file and don't quit Composer
4. open the file with another editor, no problems are found.

So, the bug should be in the function that parses the file when
the file is opened. Thats why the bug appears in "View->Page Source" 
in Navigator, because the file is parsed upon opening.

Anyone knows whats the name of the function is? 


    
    

    
  
Still happens when Composer saves the file 
Build 2002110208

    
    

    
  
Sorry, I misunderstood. You are correct that the blank lines are added at the
time Composer opens the file and not at the time Composer saves the file.

But Composer still saves the blank lines. It does not seem to occur the first
time, but modify and save the file multiple times and the blank lines will appear.


    
    

    
  
Comment #24
yes, composer saves the blank lines because the blank lines have been added to
the source when the file was opened.
What I'm trying to point out is that the blank lines are added only when
composer opens the file or when Navigator shows the source.
I want to make the original bug description more acurate.

    
    

    
  
Comments on Patch v2.0:
It seems that the patch undo was been done. I mean, the patch still lets the
extra CR to be added, and then removes the extra CR.
I have been trying to find out where the extra CR are added, but have had no
success :-(
Do you know where the extra CR are added?
I think it should be not too hard to find out, since this bug only happend when
the script/sytle tags are parsed. The problem is that I don't understand the
code so well, and after hours in lxr I could not pinpoint the exact location
where the extra CR are added.


    
    

    
  
The normalization should happen _as_we_ copy/collect the script/style content
instead of normalizing after copying. That way we can avoid/reduce the overhead.

    
    

    
  
Comment #27
Thats right. I agree. Anyone knows where the style/script collection is taking
place(what function)? So you all can start looking at the code? Thanks.

    
    

    
  
I'll work on this..
Assignee: t_mutreja → harishd
Status: ASSIGNED → NEW

    
    

    
  
*** Bug 177679 has been marked as a duplicate of this bug. ***

    
    

    
  
Summary request change:
"Composer and Navigator->ViewSource adds blank lines between SCRIPT & STYLE tags
when opening files"
For better description/scope of the bug.

On comment #29:
cool! :-)

    
    

    
  
*** Bug 177379 has been marked as a duplicate of this bug. ***

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch v3.0
        — 
          — Splinter Review
      

    


  
Convert CRLF or CR to LF whenever the skipped content is collected.

        Attachment #102124 -
        Attachment is obsolete: true

        Attachment #104366 -
        Attachment is obsolete: true

    
    

    
  
*** Bug 183995 has been marked as a duplicate of this bug. ***

    
    

    
  
Comment on attachment 107935 [details] [diff] [review]
patch v3.0

r=glazman, thanks Harish!!!

        Attachment #107935 -
        Flags: review+

    
    

    
  
Comment on attachment 107935 [details] [diff] [review]
patch v3.0

peter, can you sr please ?

        Attachment #107935 -
        Flags: review+ → review?(peterv)

        Attachment #107935 -
        Flags: superreview?(peterv)

        Attachment #107935 -
        Flags: review?(peterv)

        Attachment #107935 -
        Flags: review+

    
    

    
  
Comment on attachment 107935 [details] [diff] [review]
patch v3.0

Any idea about the perf impact?

        Attachment #107935 -
        Flags: superreview?(peterv) → superreview+

    
    

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

    
    

    
  
*** Bug 36630 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 114204 has been marked as a duplicate of this bug. ***

    
    

    
  
This bug seems to have reappeared in the last couple days.  I see it in
Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.4a) Gecko/2003040105

This includes the extra blank lines after <br>s and many other tags in BODY. 
Try the test case in Comment #6
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

    
    

    
  
If I understand this bug correctly, it is talking about whitespace added
between items in the header when the file is opened, without further edits.
Original report does not specify "Retain source formatting," but neither does
it specify "Reformat HTML."

Jim Booth's Comment 6 and testcase describes a situation when "Retain source
formatting" is selected, and edits are made in the Source view.

In the 1.4a release (2003040105), Windows 2000, I am not seeing extra space
added on file open with "Retain format" selected.  It does appear if the
source is edited.  This is identical to the current behavior being discussed
in Bug 97278 and Bug 176866.  Altho each of the three bugs discusses slightly
different areas of the original document, I submit that they are dupes, and
would grant the seniority to 97278.


PS: Note that Attachment 5 [details] appears to contain some newlines that are <CR><CR><LF>,
which results in extra lines appearing on file open.

    
    

    
  
Actually, I don't think this is a dupe of 97278; I just think that the behavior
being complained about in comment 41 (and comment 6) is described by 97278.

I would re-close this bug and redirect discussion to Bug 97278.

    
    

    
  
I'm not convinced about this one. Mainly because I don't fully understand the
original bug report. That is, did the reporter open the file into the "Normal"
window/tab and then save it, or something else?

Reporter, can you help us out here?

    
    

    
  
I am no longer seeing the original problem with extra newlines (open file in
normal then switch switch to source; no saving necessary) in build 2003040808.

Bug 97278 seems to be a separate problem.

Thanks.

    
    

    
  
Mike, I agree that problems on file open are fixed, as are added lines WITHIN
<STYLE> and <SCRIPT> sections.

I'm still seeing an added line BEFORE <STYLE> and <SCRIPT> (as well as <META>
and other places) on switching between SOURCE and NORMAL views, but I agree that
Bug 97278 does encompass this complaint, (though I wish it had a better
description.)

Reclosing this bug.  See Bug 97278 for the remaining problems
Status: REOPENED → RESOLVED
Closed: 22 years ago → 21 years ago
Resolution: --- → FIXED

    
    

    
  
A similar bug has recurred between 20030614 and 20030618 in the trunk.  
Please see Bug 210107.  It's not exactly the same, so I won't reopen this one.

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: