Bug 1059684: markdown text should not be rendered within a <pre> tag

r=glob,a=glob

7 files changed, 53 insertions, 21 deletions

diff --git a/Bugzilla/Markdown.pm b/Bugzilla/Markdown.pm
index 3023d98e2..353c2ff6a 100644
--- a/Bugzilla/Markdown.pm
+++ b/Bugzilla/Markdown.pm
@@ -99,7 +99,7 @@ sub _RunSpanGamut {
     $text = $self->_EncodeAmpsAndAngles($text);
     $text = $self->_DoItalicsAndBold($text);
 
-    $text =~ s/ {2,}\n/ <br$self->{empty_element_suffix}\n/g;
+    $text =~ s/\n/<br$self->{empty_element_suffix}\n/g;
 
     return $text;
 }
@@ -323,13 +323,6 @@ sub _DoItalicsAndBold {
     return $text;
 }
 
-# Override this function to ignore 'wrap_in_p_tags' from
-# the caller and to not generate <p> tags around the output.
-sub _FormParagraphs {
-    my ($self, $text) = @_;
-    return $self->SUPER::_FormParagraphs($text, { wrap_in_p_tags => 0 });
-}
-
 sub _DoStrikethroughs {
     my ($self, $text) = @_;
 
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index acedc65f2..fa2674366 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -632,6 +632,14 @@ sub is_bug_ignored {
     return (grep {$_->{'id'} == $bug_id} @{$self->bugs_ignored}) ? 1 : 0;
 }
 
+sub use_markdown {
+    my ($self, $comment) = @_;
+    return Bugzilla->feature('markdown')
+           && $self->settings->{use_markdown}->{is_enabled}
+           && $self->settings->{use_markdown}->{value} eq 'on'
+           && (!defined $comment || $comment->is_markdown);
+}
+
 ##########################
 # Saved Recent Bug Lists #
 ##########################
@@ -2623,6 +2631,12 @@ C<string> The current summary of the bug.
 Returns true if the user does not want email notifications for the
 specified bug ID, else returns false.
 
+=item C<use_markdown>
+
+Returns true if the user has set their preferences to use Markdown
+for rendering comments. If an optional C<comment> object is passed
+then it returns true if the comment has markdown enabled.
+
 =back
 
 =head2 Saved Recent Bug Lists
diff --git a/js/field.js b/js/field.js
index 1cd58a69a..c24603988 100644
--- a/js/field.js
+++ b/js/field.js
@@ -984,18 +984,32 @@ function initDirtyFieldTracking() {
 
 var last_comment_text = '';
 var last_markdown_cb_value = null;
+var comment_textarea_width = null;
+var comment_textarea_height = null;
 
-function show_comment_preview(bug_id) {
+function refresh_markdown_preview (bug_id) {
+    if (!YAHOO.util.Dom.hasClass('comment_preview_tab', 'active_comment_tab'))
+        return;
+    show_comment_preview(bug_id, 1);
+}
+
+function show_comment_preview(bug_id, refresh) {
     var Dom = YAHOO.util.Dom;
     var comment = document.getElementById('comment');
     var preview = document.getElementById('comment_preview');
     var markdown_cb = document.getElementById('use_markdown');
 
     if (!comment || !preview) return;
-    if (Dom.hasClass('comment_preview_tab', 'active_comment_tab')) return;
+    if (Dom.hasClass('comment_preview_tab', 'active_comment_tab') && !refresh)
+        return;
 
-    preview.style.width = (comment.clientWidth - 4) + 'px';
-    preview.style.height = comment.offsetHeight + 'px';
+    if (!comment_textarea_width) {
+        comment_textarea_width = (comment.clientWidth - 4) + 'px';
+        comment_textarea_height = comment.offsetHeight + 'px';
+    }
+
+    preview.style.width = comment_textarea_width;
+    preview.style.height = comment_textarea_height;
 
     var comment_tab = document.getElementById('comment_tab');
     Dom.addClass(comment, 'bz_default_hidden');
@@ -1029,6 +1043,12 @@ function show_comment_preview(bug_id) {
                 document.getElementById('comment_preview_text').innerHTML = data.result.html;
                 Dom.addClass('comment_preview_loading', 'bz_default_hidden');
                 Dom.removeClass('comment_preview_text', 'bz_default_hidden');
+                if (markdown_cb.checked) {
+                    Dom.removeClass('comment_preview_text', 'comment_preview_pre');
+                }
+                else {
+                    Dom.addClass('comment_preview_text', 'comment_preview_pre');
+                }
                 last_comment_text = comment.value;
                 last_markdown_cb_value = markdown_cb.checked;
             }
diff --git a/process_bug.cgi b/process_bug.cgi
index b47a3b1cf..b3d979960 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -233,8 +233,8 @@ if (should_set('keywords')) {
     $set_all_fields{keywords}->{$action} = $cgi->param('keywords');
 }
 if (should_set('comment')) {
-    my $is_markdown = ($user->settings->{use_markdown}->{is_enabled} &&
-                            $cgi->param('use_markdown')) ? 1 : 0;
+    my $is_markdown = ($user->use_markdown
+                       && $cgi->param('use_markdown') eq '1') ? 1 : 0;
 
     $set_all_fields{comment} = {
         body        => scalar $cgi->param('comment'),
diff --git a/skins/standard/global.css b/skins/standard/global.css
index 28c406b1d..60e06af73 100644
--- a/skins/standard/global.css
+++ b/skins/standard/global.css
@@ -319,7 +319,7 @@ div#docslinks {
 }
 
 /* tbody.file pre is for the Diff view of attachments. */
-.bz_comment_text, .uneditable_textarea, tbody.file pre {
+pre.bz_comment_text, .uneditable_textarea, tbody.file pre {
      font-family: monospace;
      white-space: pre-wrap;
 }
@@ -732,6 +732,10 @@ input.required, select.required, span.required_explanation {
     width: auto;
 }
 
+.comment_preview_pre {
+    white-space: pre;
+}
+
 #comment_preview_loading {
     font-style: italic;
 }
diff --git a/template/en/default/bug/comment.html.tmpl b/template/en/default/bug/comment.html.tmpl
index b748b71fd..76054f92a 100644
--- a/template/en/default/bug/comment.html.tmpl
+++ b/template/en/default/bug/comment.html.tmpl
@@ -32,14 +32,15 @@
   <div id="comment_preview" class="bz_default_hidden bz_comment">
     <div id="comment_preview_loading" class="bz_default_hidden">Generating Preview...</div>
     <div id="comment_preview_error" class="bz_default_hidden"></div>
-    <pre id="comment_preview_text" class="bz_comment_text"></pre>
+    <div id="comment_preview_text" class="bz_comment_text"></div>
   </div>
 [% END %]
 
-[% IF feature_enabled('markdown') AND  user.settings.use_markdown.value == 'on' %]
+[% IF user.use_markdown %]
   <div id="comment_markdown">
-    <input type="checkbox" name="use_markdown" id="use_markdown" value="1"
-    [% "checked=\"checked\"" IF user.settings.use_markdown.value == 'on' %] >
+    <input type="checkbox" name="use_markdown"
+           id="use_markdown" value="1" checked="checked"
+           onchange="refresh_markdown_preview([% bug.id FILTER none %])">
     <label id="use_markdown_label" for="use_markdown">Use Markdown for this [% terms.comment %]</label>
     (<a href="page.cgi?id=markdown.html" target="_blank" title="View Markdown Syntax Guide">help</a>)
   </div>
diff --git a/template/en/default/bug/comments.html.tmpl b/template/en/default/bug/comments.html.tmpl
index 617f49471..3895691d7 100644
--- a/template/en/default/bug/comments.html.tmpl
+++ b/template/en/default/bug/comments.html.tmpl
@@ -267,12 +267,12 @@
 [%# Don't indent the <pre> block, since then the spaces are displayed in the
   # generated HTML
   #%]
-<pre class="bz_comment_text[% " collapsed" IF comment.collapsed %]"
+<[% user.use_markdown(comment) ? "div" : "pre" %] class="bz_comment_text[% " collapsed" IF comment.collapsed %]"
   [% IF mode == "edit" || comment.collapsed %]
     id="comment_text_[% comment.count FILTER none %]"
   [% END %]>
   [%- comment_text FILTER markdown(bug, comment) -%]
-</pre>
+</[% user.use_markdown(comment) ? "div" : "pre" %]>
     [% Hook.process('a_comment-end', 'bug/comments.html.tmpl') %]
     </div>
 [% END %]