Closed Bug 32000 Opened 26 years ago Closed 25 years ago

Extra newlines in comments made with NS4-Mac

Categories

(Bugzilla :: Bugzilla-General, defect, P3)

Product:
Bugzilla
Component:
Bugzilla-General
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Bugzilla 2.12

People

(Reporter: BenB, Assigned: justdave)

References

Details

(Whiteboard: [dogfood-][PDT-] 2.12)

Attachments

(2 files)

Propsed fix (no quoting of EOL char in value_quote())
985 bytes, patch
Details | Diff | Splinter Review
New patch that follows the recommendation made in the previous comment.
667 bytes, patch
Details | Diff | Splinter Review
Bugzilla adds illegal CR into the hidden input field, that is used, if a login is required after commit. This causes comments like this one. May be a Linux-specific bug. See recent comments in bug #22983 for details.
Terry? This is pretty awful and should be easy to fix.
Keywords: dogfood
I managed to fix this in my local copy by changing the value_quote() routine in CGI.PL to change \n\r to \n first before quoting. And leave the \n as is, don't quote it as 
. Spanning lines is legal in HTML, as long as it's inside the quotes it will preserve it as part of the value. The HTTP/1.1 spec says that, when dealing with anything with a text/xxxx mime-type, the entire document (including the contents of values) must have a consistent end-of-line character. This basically means that if the server is Linux based, it needs to be a LF. The HTTP/1.1 spec also says that any ocurrance of a LF by itself, a CR by itself, or a CRLF should be interpreted as a single line break. Since in most of these cases, it's coming from a Linux server, the browsers are assuming it's LF, and treating them all individually, causing a duplicate when you have CRLF. This is avoided by not encoding the end-of-line characters, but leaving them as \n. Perl and C both define \n as the current system's end-of-line character. This means it isn't necessarily a LF, but will always be a line break. I'm still testing it right now... if I get some more testing done and I'm happy with it, I'll upload a diff.
OK, Done some more testing, I'm happy with it. It not only eliminated the periodic double-spacing in the new comments, but also eliminated the supposedly "empty" fields in editparams that wound up with blank lines in them. Diff coming right up.
Dave, tnx for fixing this! CCing Dawn, who had problams with editparams.
This fixed display problems in editparams when viewing with an HTML-compliant browser (such as iCab on the Mac), but apparently did not fix the problem with changes to textareas being falsely detected... but it *does* solve the double- spaced comments problem.
Putting on PDT- for beta1 dogfood.
Whiteboard: [PDT-]
How are dogfood and PDT- relevant to a Bugzilla bug?
Matty, DOGFOOD means, that I can/do use Mozilla for my daily work. This bug discourages that. Strictly, "DOGFOOD", "beta1" etc. are limited to Netscape. Neither me nor terry belong to it, so you're right in part. But now that we haye these well-known keywords, I think, they're useful for Mozilla-development, too. E.g. I sometimes search for beta1/dogfood and pdt- to look for serious bugs, which may not be addressed. leger, this bug was never nominated for beta1 :).
But this is not a bug with Mozilla. It implies dogfood for using Bugzilla.
tara@tequilarista.org is the new owner of Bugzilla and Bonsai. (For details, see my posting in netscape.public.mozilla.webtools, news://news.mozilla.org/38F5D90D.F40E8C1A%40geocast.com .)
Assignee: terry → tara
Severity: normal → major
Any progress on this bug? I did some mozilla testing, but after the first few tries I decided not to use mozilla for bug reports or comments because of this problem. It seems that there's a proposed fix attached. What's wrong with it?
hello?
Based on recent comments here, I'd say the mozilla.org people are getting really anxious to see this patch checked in. Tara??
Hmm, actually, those guys haven't posted to this bug in a while, but I'm sure they want it too. But several other people are anxious to see it checked in, regardless... :)
Status: NEW → ASSIGNED
Putting on [dogfood-] radar. Not a blocker to daily build.
Whiteboard: [PDT-] → [dogfood-][PDT-]
Bug 22983 was recently fixed and should fix the symptoms you're seeing here, I think. I haven't actually tested this yet. See bug 4928 for rationale behind value_quote. What its doing is not illegal. Marking this bug invalid. If you still disagree or if the 22983 fix didn't actually work then please reopen and bring Ian Hickson and/or David Baron into this bug. I added a comment to the value_quote code in CGI.pm referencing these bugs.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → INVALID
OK, let's find out if it's fixed... I'm not logged in right now. After I click Commit on this bug, I'll be presented with a login box. After I log in and the update has been submitted, if this has been fixed, this comment will be nice and neat. If it has not been, this comment will be double-spaced.
Nope, it's not fixed. I don't have the authority to reopen this bug. Someone who can, please do so. Thanks.
You sure you don't want to reopen bug 22983 instead? That's the one that Eric thought he fixed yesterday, and it's exactly the scenario described here (though I still think that using the non-DOM-compatible CRLF instead of LF when remembering hard returns seems like bad practice).
This isn't a Mozilla problem, it's a Bugzilla problem. I can duplicate this on every browser I have (Netscape Communicator 4.7, iCab 1.9a, Internet Explorer 4.01). bug 22983 is a Mozilla bug. Reopen this one.
Okay, I'll reopen it for you (if nothing else, there's the dom compliance issue -- dom newlines vs platform newlines). But I think you're the first person to report seeing this on anything other than mozilla (I assume you're on Mac, since you mention icab?) so it might help to give more info about your platform. I use 4.73/Linux with bugzilla all the time and don't see this problem.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Adding David Baron and Ian Hickson to the CC, per Dawn's request. Actually, now that I'm looking at it, this is a duplicate of 4928, and it's *not* fixed. Ian Hickson's 1999-4-14 comment lists what the correct solution is (which is not what I did with my patch that's attached to this bug), and it is also not what Bugzilla is currently doing. Bugzilla is still escaping the newlines as &013;&010; when it should be escaping them simply as &013;. And yes, you surmise correctly that I'm using a Mac. I do have a PC at my disposal here (with Win98, Linux, and BeOS on it) so I can experiment with it on a few platforms if you'd like. :) The Mac uses just CR for EOL. BeOS and Linux both use just LF. Windows uses CRLF.
Here's another test. Netscape 4.61/Win98. I haven't logged in yet from this IP, so I'm going to be prompted for the login box after I click Commit. Once again, we're checking to see if this gets double-spaced or not (sorry for repeating myself so much, mostly just filling up space so I have am multi-line comment).
OK, looks like Netscape/Win98 is fine... Here's a shot with Internet Explorer 4.0 (4.72.something is the real version number) on Win98. One more line for good measure and off to Commit and login..
Internet Explorer/Win98 did good, too. Here goes with NetPositive 2.2 on BeOS 5... a few more things to say to make this a multiline comment (sorry for the spam everyone, you can ignore this if you want). OK that's enough, off to commit and login again.
Ah, swift... NetPositive doesn't support WRAP=HARD. :) ok, let's try this again, I'llput the EOL's in myself. I just logged out, now I'll hit Commit again and hope I geta login prompt again.
OK, NetPositive obviously sucks. :) I've never gotten around to getting PPP set up on the Linux partition, working on that now. If and when I get it working, I'll try it from there.
Dave, don't worry, Netscape Nav. 4.7x doesn't show the problem.
On Linux, that is.
OK, here's a shot with iCab 1.9a (Mac) just to make sure I wasn't dreaming it before. Inserting my own CRs because I know for a fact iCab doesn't support WRAP=HARD, but I'm pretty sure it'll preserve the EOL in the textboxes...
Oooohhh, and iCab does it fine. Looks like a bug in the Mac version of Netscape then....
I always liked iCab better anyway (it follows the standards better than Netscape does for one). The only thing I use Netscape for is accessing Bugzilla because it uses too many Netscape/Internet Explorer non-standard extensions (like WRAP= HARD for example). iCab's built-in HTML checker tells me this: Warning (116/1): The attribute "WRAP" is not allowed for the tag <TEXTAREA>. (FYI, I'm using Netscape again to post this one)
OK, there were a couple of proposed fixes to this in this bug and the other one... do any of those fixes break any other browsers? If not, could one of them be applied anyway, to cater to Netscape Mac? Of course, it would also make me happy if Bugzilla stopped using WRAP=HARD and just did the line-wrapping on the server-side when it gets POSTed. Then I could stop using Netscape to access Bugzilla. :)
assigning to me for possible inclusion into 2.12
Assignee: tara → cyeh
Status: REOPENED → NEW
Whiteboard: [dogfood-][PDT-] → [dogfood-][PDT-] 2.12
*** Bug 24878 has been marked as a duplicate of this bug. ***
so i was going to check this in, but it appears this has already been committed to the trunk on revision 1.67 by endico. marking fixed.
Status: NEW → RESOLVED
Closed: 25 years ago → 25 years ago
Resolution: --- → FIXED
What she committed in 1.67 was a comment indicating that this shouldn't be fixed. It hasn't been fixed. (And I reopened this after she marked it fixed when she checked it in, too, if you read the above comments) If I add a comment from the Mac version of Netscape Navigator while not being logged in yet, it still double-spaces it. Besides the patch posted to this bug, there are a few other methods of trying to fix it also listed in one of the other bugs referenced here. I posted a request here for people to try them to see if they worked as expected on other browsers, because if any one of them produced positive results on all of the browsers, it should be put in. Based on the comments, no one has done that yet. While on the one hand I hate to cater to what's obviously a browser bug, the particular browser in question is very widely used, and it's making a mess of comments all over everyone's Bugzilla installations.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
i'm sorry, i'm a complete and utter buckethead. you're right. i've been apologizing a lot to everyone lately.
I have no idea on how to fix this.
As well as when logging in, this happens on any other situation where you go through an intermediary page before committing your comment: * when a mid-air collision is detected, but you say `Commit my changes anyway' * when you change the product, and therefore have to change the component.
dave, could you resummarize this bug and update the summary? We're trying to re-evaluate the bug which is hard because parts of this have changed since the time the bug was first reported. We're leaning towards wontfix here but maybe we're wrong. I think the summary should now be "Extra newlines in comments made with ns4.x on mac". From what I can tell, bugzilla's handling of line endings is now correct (bug 4928) and you're asking for it to be re-broken in order to work around a bug on certain browsers. Is that right? I guess if you still want this fix you should restate your case. Would applying your fix break bug 3474 or otherwise bust other people? Ok, i think i'm finally making sense out of mpt's "as well as" comment. I think I think he means that there are extra newlines in comments as well as in hidden fields. I vaguely remember the problem and can't remember which field it was. I think it was some field that had nothing in it but a broken line break. I can't remember what problem it caused.
After reviewing the current code and the text of bug 4928, I have come to the conclusion that the fix applied in bug 4928 has since been broken, and had that fix stayed in place this would not be an issue. Below is the code from CGI.pl that affects this: # See bug http://bugzilla.mozilla.org/show_bug.cgi?id=4928 for # explanaion of why bugzilla does this linebreak substitution. # This caused form submission problems in mozilla (bug 22983, 32000). $var =~ s/\n/\&#010;/g; $var =~ s/\r/\&#013;/g; According to the text in bug 4928, what is supposed to be happening is that all CRLF pairs and all lone LFs are supposed to be getting converted to CR only. Based on the descriptions given in bug 4928, that code should read as follows: $var =~ s/\r\n/\&#013;/g; $var =~ s/\n\r/\&#013;/g; $var =~ s/\r/\&#013;/g; $var =~ s/\n/\&#013;/g; I tried this out on my installation, using the following browsers: NS4.7 Mac, NS4.5 Linux, MSIE 4.5 Mac, MSIE 5.0 Windows and this does indeed work on all of the above browsers.
Summary: Extra newlines in comments made with Mozilla → Extra newlines in comments made with Mozilla in NS4-Mac
This is a relatively simple fix, and I am about 95% positive it's not going to break anything. Anyone want to try it out on a test install anywhere or shall I just go ahead and check it in? I'm forgot to log in first before making a comment on a bug tonight, and I ran into this again on b.m.o.
Assignee: cyeh → dave
Status: REOPENED → NEW
fixing summary. need a review on the patch...
Summary: Extra newlines in comments made with Mozilla in NS4-Mac → Extra newlines in comments made with NS4-Mac
r=timeless
Keywords: approval, patch
OS: Linux → All
Hardware: PC → All
checked in.
Status: NEW → RESOLVED
Closed: 25 years ago → 25 years ago
Resolution: --- → FIXED
Sorry for the spam, but I needed to be able to query for all of these correctly.
Target Milestone: --- → Bugzilla 2.12
*** Bug 71384 has been marked as a duplicate of this bug. ***
*** Bug 71384 has been marked as a duplicate of this bug. ***
Moving closed bugs to Bugzilla product
Component: Bugzilla → Bugzilla-General
Product: Webtools → Bugzilla
Version: other → unspecified
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: