313534 - Bugzilla wraps old comments too eagerly

Status: RESOLVED WORKSFORME

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[[e-mail]]

If you look at bug 295941 comment 0 or bug 313155 comment 7 you'll see that a word on the first line of the comment has been wrapped on to its own line...

Comment 1

[Max Kanat-Alexander]

These can be fixed individually, but they're basically the result of a heuristic that I used to determine if a comment was "mis-wrapped." I believe the test is "if the first line is longer than 80 characters, the comment is mis-wrapped."

I recall that was going to make that "if the first line is longer than 83 characters" but it looks like I didn't do that. I could do that. But it won't fix the problem on bmo.

To fix the problem on bmo, you'd have to look for all instances of longdescs.thetext where the first line was longer than 80 characters but shorter than 85, and set already_wrapped to 1.

Comment 2

[Olav Vitters]

The heuristic is wrong. Quoting the relevant code:
>    # If an old comment doesn't have a newline in the first 80 characters,
>    # (or doesn't contain a newline at all) and it contains a space,
>    # then it's probably a mis-wrapped comment and we should wrap it
>    # at display-time.
>    print "Fixing old, mis-wrapped comments...\n";
>    $dbh->do(q{UPDATE longdescs SET already_wrapped = 0
>                WHERE (} . $dbh->sql_position(q{'\n'}, 'thetext') . q{ > 80
>                   OR } . $dbh->sql_position(q{'\n'}, 'thetext') . q{ = 0)
>                  AND SUBSTRING(thetext FROM 1 FOR 80) LIKE '% %'});

The comment box is 80 characters wide. If such a sentence is exactly 80 characters wide, the newline character will be on character position 81. E.g. within the the SQL the '> 80' should be '> 81'.

There will still be a few cases with users saying 'Hi!' on the first row, then producing a very wide comment.. but not sure how to detect those easily.

Feel free to reassign to me.

Comment 3

[Frédéric Buclin]

Only security and dataloss fixes will be accepted on the 2.20 branch.

Comment 4

[Gervase Markham [:gerv]]

Max, Olav: is this still worth fixing?

Gerv

Comment 5

[Max Kanat-Alexander]

Actually, I ended up fixing this during the checksetup.pl re-write, just since I was in there moving that code anyway.

Comment 6

[Mike Cowperthwaite]

Bug 253403 comment 0 & 1 seem to exhibit the problem of this bug; I haven't been searching for others.  I suppose it's possible those two comments were stored with hard line breaks at those positions...?  Max, do you also see this?

Comment 7

[Max Kanat-Alexander]

(In reply to comment #6)
> Bug 253403 comment 0 & 1 seem to exhibit the problem of this bug; I haven't
> been searching for others.  I suppose it's possible those two comments were
> stored with hard line breaks at those positions...?  Max, do you also see this?

  bmo had this problem when it upgraded, and so the problem will always exist on bmo unless repaired manually. This bug was only fixed for people who upgrade from 2.18 to 3.0.