Closed Bug 526734 Opened 15 years ago Closed 15 years ago

English string "From update of attachment" in localized installations

Categories

(Bugzilla :: Attachments & Requests, defect)

defect
Not set
minor

Tracking

()

RESOLVED FIXED
Bugzilla 3.6

People

(Reporter: mkanat, Assigned: mkanat)

References

Details

(Keywords: l12y)

Attachments

(1 file, 1 obsolete file)

bug 317127 handles the "Created an attachment" message, but the "From update of attachment" message still exists as a hardcoded string in attachment.cgi.
Attached patch v1 (obsolete) — Splinter Review
In addition to fixing this bug, this does a few nice things:

1) It changes the message to "Comment on attachment X", which I think is a bit nicer.

2) It shows the description of the comment below the "Comment on attachment X" header, which I think is useful because I never have any idea what attachment is being commented *on*, when people leave attachment comments.
Assignee: attach-and-request → mkanat
Status: NEW → ASSIGNED
Attachment #410470 - Flags: review?(LpSolit)
Oh, and the patch of course requires the patch from the blocker.
Depends on: 452919
No longer depends on: 317127
Keywords: l12y
Attachment #410470 - Flags: review?(LpSolit) → review-
Comment on attachment 410470 [details] [diff] [review]
v1

The blocker has been updated meanwhile, and this patch no longer applies cleanly.
Whiteboard: [needs new patch asap]
Attached patch v2Splinter Review
Okay, here we go.
Attachment #410470 - Attachment is obsolete: true
Attachment #416442 - Flags: review?(LpSolit)
Whiteboard: [needs new patch asap]
Comment on attachment 416442 [details] [diff] [review]
v2

>Index: Bugzilla/Install/DB.pm

>+sub _set_attachment_comment_type {

>+    my $what = "update";
>+    if ($type == CMT_ATTACHMENT_CREATED) {
>+        $what = "creation";
>+    }

Nit: my $what = ($type == CMT_ATTACHMENT_CREATED) ? 'creation' : 'update'; would be cleaner IMO.


>+        next if $text !~ /\Q$string\E(\d+)/;

Nit: would be more robust as /^\Q$string\E(\d+)/ I think, so that we don't catch unrelated strings in the middle of a comment.


>+sub _set_attachment_comment_types {

>+    my %comment_ids = map { $_ => 1 } (@{ $created_ids || [] },
>+                                       @{ $updated_ids || [] });

A comment cannot be of both types, so
  my @comment_ids = (@$created_ids, @$updated_ids);
would be cleaner. Just replace
  return if !scalar @comment_ids;
by
  return [] if !scalar @comment_ids;
in _set_attachment_comment_type().


r=LpSolit
Attachment #416442 - Flags: review?(LpSolit) → review+
Flags: approval+
Checking in attachment.cgi;
/cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v  <--  attachment.cgi
new revision: 1.165; previous revision: 1.164
done
Checking in Bugzilla/Comment.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Comment.pm,v  <--  Comment.pm
new revision: 1.3; previous revision: 1.2
done
Checking in Bugzilla/Constants.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Constants.pm,v  <--  Constants.pm
new revision: 1.124; previous revision: 1.123
done
Checking in Bugzilla/Install/DB.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/DB.pm,v  <--  DB.pm
new revision: 1.78; previous revision: 1.77
done
Checking in template/en/default/bug/format_comment.txt.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/format_comment.txt.tmpl,v  <--  format_comment.txt.tmpl
new revision: 1.7; previous revision: 1.6
done
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: