1093924 - Move replyToMarkdownComment() and replyToComment() out of templates

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, enhancement)

Description

[Frédéric Buclin]

The replyToMarkdownComment() JS function is not context dependent and so doesn't need to be in bug/comments.html.tmpl. It should be moved into js/comments.js.

Comment 1

[David Lawrence [:dkl]]

Attachment: 1093924_1.patch

Also moved replyToComment to js/comments.js as well.

dkl

Comment 2

[Frédéric Buclin]

Comment on attachment 8517585 [details] [diff] [review]
1093924_1.patch

>diff --git a/js/comments.js b/js/comments.js

>+function replyToComment(id, real_id, name, text) {
>+    var prefix = "(In reply to " + name + " from comment #" + id + ")\n";

This string is no longer localizable. It should still be defined in the template and then passed to this JS function.


>+    if (replyCommentConfig.is_insider) {

Below, you set: is_insider : [% user.is_insider ? "true" : "false" %]. Are you sure JS correctly understands "true" and "false" as booleans instead of strings? I didn't test your patch yet.



>+function replyToMarkdownComment(id, real_id, name) {

>+    textarea.value += "Fetching comment...";

Same problem here. The string is no longer localizable.



>+++ b/template/en/default/bug/comments.html.tmpl

>+  var replyCommentConfig = {
>+      quote_replies : "[% user.settings.quote_replies.value FILTER js %]",
>+      is_insider : [% user.is_insider ? "true" : "false" %]
>+  };

Wouldn't it be better to pass 1 or 0 instead of true/false?

Comment 3

[David Lawrence [:dkl]]

Attachment: 1093924_2.patch

Comment 4

[Frédéric Buclin]

Comment on attachment 8523942 [details] [diff] [review]
1093924_2.patch

>+++ b/js/comments.js

>+function replyToComment(id, real_id, replyto_header, text) {

>+    } else if (replyCommentConfig.quote_replies == 'simple_reply') {
>+        replytext = prefix;
>+    }

s/prefix/replyto_header/


>+function replyToMarkdownComment(id, real_id, replyto_header, name) {

The 4th argument 'name' is unused. Please remove it.


r=LpSolit with these fixes on checkin.

Comment 5

[David Lawrence [:dkl]]

To ssh://[e-mail]/bugzilla/bugzilla.git
   0bc389f..be26c11  master -> master