Closed Bug 210107 Opened 23 years ago Closed 23 years ago

When you switch from normal to html, composer doubles blank lines in head

Categories

(SeaMonkey :: Composer, defect)

Product:
SeaMonkey
Component:
Composer
Platform:
x86
Windows 98
Type:
defect
Priority:
Not set
Severity:
major

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
mozilla1.5beta

People

(Reporter: Rstetson, Assigned: brian)

Details

Attachments

(2 files, 1 obsolete file)

Test Case to demonstrate the problem
1.09 KB, text/html
Details
Fix v. 2
1.36 KB, patch
Brade
: review+
tor
: superreview+
Details | Diff | Splinter Review
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.5a) Gecko/20030618 Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.5a) Gecko/20030618 it adds blank lines which does not effect the view when you see the final product but which does effect the html code. when you try to copy the code to the web, the document is immense with many blank lines. It makes uploading and editing a major PIA! Reproducible: Always Steps to Reproduce: 1. edit code in normal mode 2. switch to html 3. repeat several times 4. watch as your efforts get bigger and bigger and bigger Actual Results: your file grows Expected Results: it should not add all those blank lines in between lines of code
Added a test case. Blank lines are being added in the <STYLE> AND <SCRIPT> sections in the <HEAD> The setting of "Reformat HTML source" or "Retain Original Source Formatting" in Composer Preferences does NOT affect this. (It happens in either mode.) Each time you switch out of HTML Source mode, the number of blank lines is DOUBLED PLUS ONE (1,3,7,15,31,63...)
This is similar to Bug 159615 which was fixed in April. Confirming in Build 2003061207 and CCing harishd@netscape.com, brade@netscape.com, glazman@netscape.com Problem has appeared between build 20030614 and 20030618. This makes COMPOSER UNUSABLE if you use scripts or style sheets in the head!
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Whenever you switch from normal to html it adds 2 to 8 blank lines → When you switch from normal to html, composer doubles blank lines in head
I do not see this problem in the 6/18 3am macho build from mozilla. Is it Windows-specific? Can we get a narrower and/or more accurate timeframe when this regressed?
I wouldn't be surprised if this is Windows specific. In the past these problems have resulted from converting CR/LF to LF and back. In my comment #2, I gave the wrong build number. I can confirm it in 2003062307. I know it was OK in 20030614 and reporter found it on 20030618, so it regressed between 6/14 and 6/18.
The problem is in AppendToStringConvertLF. http://lxr.mozilla.org/seamonkey/source/content/base/src/nsHTMLContentSerializer.cpp#1013 If I replace the code in there with AppendToString(aStr, aOutputStr); then the problem is solved. I'll do some more investigation.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.5beta
I found the true source of them problem now. The source editor gets CRLF linebreaks, but is designed to work with just LF linebreaks. I can see some minor glitches from that. For example, place the caret at the beginning of a line and press backspace twice. See what happens as you delete both the CR and LF. You can also go to the end of a line and press delete twice. If you put a linebreak in yourself, it doesn't take two deletes to get rid of it. When the source is parsed again, two linebreaks are generated in the DOM. This is also what's causing blank lines over the entire document when "Retain original source formatting" is on. Now I just need to figure out where the DOM is serialized into the source editor in Composer.
Attached patch Fix (obsolete) — — Splinter Review
It's just a matter of setting the OutputLFLineBreak flag when writing out the source to the editor. Without any flags specifying the linebreak format, the serializer uses the current platform's linebreaks as a default. Good for outputting to files, but bad if the result is used inside Mozilla. The first change is where it made a difference. I went ahead and added the flag in the two other places where outputToString was called. (Neither has to do with saving to disk.)
Comment on attachment 128050 [details] [diff] [review] Fix Ready for review. I double-checked that files are still being saved with CRLF linebreaks on Windows.
Attachment #128050 - Flags: review?
Attachment #128050 - Flags: review? → review?(petersen)
Attachment #128050 - Flags: review?(petersen) → review?(brade)
Attached patch Fix v. 2 — — Splinter Review
A couple more comments, and a spelling fix in a nearby comment, as suggested by brade.
Attachment #128050 - Attachment is obsolete: true
Comment on attachment 128055 [details] [diff] [review] Fix v. 2 r=brade
Attachment #128055 - Flags: superreview?(tor)
Attachment #128055 - Flags: review+
Actually assigning to myself.
Assignee: composer → burpmaster
Status: ASSIGNED → NEW
Status: NEW → ASSIGNED
Attachment #128055 - Flags: superreview?(tor) → superreview+
Requesting approval for 1.4 branch... Who can check this into the trunk?
Flags: blocking1.4.x?
This is still not checked into the Trunk. Can someone help us out?
Was this patch tested on Linux **AND** Windows **AND** Macintosh ? Is answer is yes, I'll be glad to land it for you.
Attachment #128050 - Flags: review?(brade)
It's only been tested on Windows, but I don't think the risk is very high. This patch just makes sure that we're sending strings with LF linebreaks to a couple places where they're expected, instead of using whatever the current platform's linebreak format happens to be. The plaintext editor wants LF linebreaks. The HTML parser doesn't really care, but if we give it CRLF linebreaks, we run into bug 190955. So giving LF linebreaks in these cases doesn't change anything on Linux and shouldn't be a problem on MacOS, but if you want testing, we'll have to ask around, because there probably aren't any Mac users watching this Windows-only bug.
This isn't blocking 1.4.1. When it's got proper reviews (and landed on the trunk) please request approval1.4.x? on the patch and drivers will consider it for a 1.4.x release.
Flags: blocking1.4.x? → blocking1.4.x-
Fixed. You can try out the latest nightly. The last major bug like this is covered in bug 97278 and only affects "Retain original source formatting" mode. It is greatly reduced by this fix, and is next on my hitlist.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: