This report stems from the following webkit-dev e-mail: https://lists.webkit.org/pipermail/webkit-dev/2010-April/012528.html
This is configured here: http://trac.webkit.org/browser/trunk/BugsSite/Bugzilla/Constants.pm?rev=45520#L263
Hmm, I guess it would be best if we can also make the box's wrapping behave dynamically based on browser window size, etc. (when reading comments). Otherwise, increasing the limit to 120, say, will force all flowing comment text to be at least that wide.
(In reply to comment #2) > Hmm, I guess it would be best if we can also make the box's wrapping behave > dynamically based on browser window size, etc. (when reading comments). > Otherwise, increasing the limit to 120, say, will force all flowing comment > text to be at least that wide. My thought is that maybe the comments field can be just white-space: pre-wrap instead of white-space: pre, and therefore wrap to the window width. That way we shouldn't need hard line breaks at all.
I didn't know until looking at the code that Bugzilla already doesn't wrap lines if they begin with ">": http://trac.webkit.org/browser/trunk/BugsSite/Bugzilla/Util.pm?rev=45520#L367 For example, 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 >23456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 Perhaps the fix can be there (simply always execute the "if" block).
(In reply to comment #4) > Perhaps the fix can be there (simply always execute the "if" block). Ignore that last line. I just thought it was interesting that Bugzilla doesn't wrap lines that begin with ">". Maybe people already knew that.
Created attachment 54781 [details] Proposed patch I'm not 100% sure that increasing the COMMENT_COLS constant won't affect other forms on the site that I might be less familiar with. Nevertheless, I wanted to get a candidate patch out there.
(In reply to comment #3) > My thought is that maybe the comments field can be just white-space: pre-wrap > instead of white-space: pre, and therefore wrap to the window width. That way > we shouldn't need hard line breaks at all. By the way, note that the comments field is already "white-space: pre-wrap": .bz_comment_text, .uneditable_textarea { font-family: monospace; /* Note that these must all be on separate lines or they stop working in Konqueror. */ white-space: pre-wrap; /* CSS 3 & 2.1 */ white-space: -moz-pre-wrap; /* Gecko */ white-space: -pre-wrap; /* Opera 4-6 */ white-space: -o-pre-wrap; /* Opera 7 */ } It seems to be the width styling and the quoted lines' "white-space: pre"-- span.quote { color: #65379c; /* Make quoted text not wrap. */ white-space: pre; } that are both preventing the pre-wrap style from taking effect:
Any takers? I'm assuming this change will go live when it lands. Assuming that, I will be online when it lands to check for any unusual or undesired behavior, etc.
Comment on attachment 54781 [details] Proposed patch > diff --git a/BugsSite/Bugzilla/Constants.pm b/BugsSite/Bugzilla/Constants.pm > # The column length for displayed (and wrapped) bug comments. > -use constant COMMENT_COLS => 80; > +# > +# We choose a large value to prevent Text::Wrap::wrap() from inserting > +# hard line breaks. This allows us to use "white-space: pre-wrap" > +# in the CSS for line-wrapping instead. > +use constant COMMENT_COLS => 160; Isn't this going to cause funny wrapping if someone types more than 160 characters in a single "paragraph"? Is there a way to disable the wrapping altogether (e.g., setting COMMENT_COLS to 0 or -1 or modifying code elsewhere)? > diff --git a/BugsSite/skins/standard/global.css b/BugsSite/skins/standard/global.css > index aca0ab5..1ef62e9 100644 > --- a/BugsSite/skins/standard/global.css > +++ b/BugsSite/skins/standard/global.css > @@ -260,7 +260,8 @@ div#docslinks { > } > > .bz_comment_text { > - width: 50em; > + /* Do not specify a width. Otherwise, comment lines will "pre-wrap" > + to the specified width rather than to the window width. */ > } > > .bz_first_comment { > @@ -275,6 +276,14 @@ div#docslinks { > padding: 1em 0; > } > > +.bz_comment_text > span.quote { > + /* Prevent "white-space: pre" from styling quoted lines inside comments. > + Otherwise, a single quoted line that extends beyond the window > + width will cause all non-quoted long lines to extend beyond the > + window width as well. */ > + white-space: inherit; > +} > + > span.quote { > color: #65379c; > /* Make quoted text not wrap. */ All of these changes need to be made to skins/custom/global.css, not skins/standard/global.css, since the custom version overrides the standard version. (It's how Bugzilla rolls.)
(In reply to comment #9) Thanks, David. > > +use constant COMMENT_COLS => 160; > > Isn't this going to cause funny wrapping if someone types more than 160 > characters in a single "paragraph"? Is there a way to disable the wrapping > altogether (e.g., setting COMMENT_COLS to 0 or -1 or modifying code elsewhere)? Oops. Yes, of course. We're using this for text and not just code. I experimented with this a bit and found that Perl's $Text::Wrap::wrap() function automatically increases values of -1 and 0 to a minimum of 2. The maximum supported value seems to be 32766, so another approach could be to set the value high. It looks like the code enforces a maximum comment length that is roughly double that: > # The column length for displayed (and wrapped) bug comments. > use constant COMMENT_COLS => 80; > # Used in _check_comment(). Gives the max length allowed for a comment. > use constant MAX_COMMENT_LENGTH => 65535; In code seems like it may be the cleanest approach. BugsSite/Bugzilla/Util.pm contains a wrap_comment() subroutine (which uses COMMENT_COLS) which suggests that no other code will be affected by a change there.
(In reply to comment #10) > The maximum supported value seems to be 32766, so another approach could be to > set the value high. It looks like the code enforces a maximum comment length > that is roughly double that: > > > # The column length for displayed (and wrapped) bug comments. > > use constant COMMENT_COLS => 80; > > # Used in _check_comment(). Gives the max length allowed for a comment. > > use constant MAX_COMMENT_LENGTH => 65535; > > In code seems like it may be the cleanest approach. BugsSite/Bugzilla/Util.pm > contains a wrap_comment() subroutine (which uses COMMENT_COLS) which suggests > that no other code will be affected by a change there. That seems fine (setting the value high).
Created attachment 55246 [details] Proposed patch 2 Addressed comments. Do changes to the bugs site go live when they land, or is there a manual step?
They should update automatically... if they do not, ping me on irc (_wms) or email me.
Comment on attachment 55246 [details] Proposed patch 2 Looks good. Lets give this a try. r=me
(In reply to comment #6) > I'm not 100% sure that increasing the COMMENT_COLS constant won't affect other > forms on the site that I might be less familiar with. After taking a closer look at this before landing, it looks like changing COMMENT_COLS will affect other areas of the site. Namely, COMMENT_COLS seems to control the width of the textareas used to input comments, and not just the wrapping behavior for comment output. So it seems like we should create a new constant that we know will affect only comment display. Will post patch shortly.
Created attachment 55334 [details] Proposed patch 3 This patch is the same as before, but creates COMMENT_COLS_WRAP exclusively for use in wrap_comment(). That way we can leave COMMENT_COLS alone for other parts of the code.
Comment on attachment 55246 [details] Proposed patch 2 Cleared Darin Adler's review+ from obsolete attachment 55246 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment on attachment 55334 [details] Proposed patch 3 r=me
Comment on attachment 55334 [details] Proposed patch 3 Clearing flags on attachment: 55334 Committed r59047: <http://trac.webkit.org/changeset/59047>
All reviewed patches have been landed. Closing bug.
Test: 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 > 34567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890
(In reply to comment #10) Test: > I experimented with this a bit and found that Perl's $Text::Wrap::wrap() function automatically increases values of -1 and 0 to a minimum of 2. > > The maximum supported value seems to be 32766, so another approach could be to set the value high. It looks like the code enforces a maximum comment length that is roughly double that: > > > # The column length for displayed (and wrapped) bug comments. > > use constant COMMENT_COLS => 80; > > # Used in _check_comment(). Gives the max length allowed for a comment. > > use constant MAX_COMMENT_LENGTH => 65535; > > In code seems like it may be the cleanest approach. BugsSite/Bugzilla/Util.pm contains a wrap_comment() subroutine (which uses COMMENT_COLS) which suggests that no other code will be affected by a change there.