Closed Bug 159615 Opened 22 years ago Closed 22 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
(deleted), text/html
Details
Additional test case showing more problems switching between SOURCE & NORMAL views
(deleted), text/html
Details
testcase for both STYLE and SCRIPT bugs
(deleted), text/html
Details
patch v3.0
(deleted), 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 (deleted) —
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) (deleted) — — 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) (deleted) — — 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 (deleted) — — 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 → 22 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: