[webkit-reviews] review denied: [Bug 37792] bugs.webkit.org: should not hard-wrap comment lines exceeding 80 characters : [Attachment 54781] Proposed patch

[bugzilla-daemon at webkit.org]

David Kilzer (ddkilzer) <ddkilzer at webkit.org> has denied Chris Jerdonek
<cjerdonek at webkit.org>'s request for review:
Bug 37792: bugs.webkit.org: should not hard-wrap comment lines exceeding 80
characters
https://bugs.webkit.org/show_bug.cgi?id=37792

Attachment 54781: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=54781&action=review

------- Additional Comments from David Kilzer (ddkilzer) <ddkilzer at webkit.org>
> 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.)