290972 - In the "Find a Specific Bug" page, the status of the bug being searched cannot be translated

Status: RESOLVED FIXED

(Bugzilla :: User Interface, defect)

Description

[Emmanuel Seyman]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; fr-FR; rv:1.7.6) Gecko/20050323 Firefox/1.0.2 Fedora/1.0.2-1.3.1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; fr-FR; rv:1.7.6) Gecko/20050323 Firefox/1.0.2 Fedora/1.0.2-1.3.1

The "Find a Specific Bug" page allows the users to search for a bug with a
specific status. Due to the fact that the status shown to the user is the same
one being used in the request, this forces the english terms to be used, even if
the user is using another language (e.g. french).

The term used by the CGI scripts should be in English but the user should be
able to view the choices in whatever language he is using.

Reproducible: Always

Steps to Reproduce:
1. Use Bugzilla in any language other than English 
2. Click on 'Search' in the footber
3. Click on "Find a Specific Bug"
4. Look at the list of statuses in the 'Status' box

Actual Results:  
Choices are 'open', 'closed' and 'all'

Expected Results:  
Choices should be in the same language as the rest of the templates

Comment 1

[Emmanuel Seyman]

Attachment: Converts array s into a hash with statuses and localized equivalents

Comment 2

[Emmanuel Seyman]

Attachment: Converts array s into a hash with statuses and localized equivalents

Note to self: run runtests.pl BEFORE submitting the patch.
This replaces the s variable in filterexceptions.pl with the key variable.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 181150 [details] [diff] [review]
Converts array s into a hash with statuses and localized equivalents

Good idea, but the hash won't preserve the order of the items, which we want to
do.

Comment 4

[Myk Melez [:myk] [@mykmelez]]

You could have an array of keys along with the key->localized value mapping, but
perhaps there's a better way. cc:ing Gerv, who knows more about Bugzilla l10n,
for his opinion.

Comment 5

[Gervase Markham [:gerv]]

The issue with:
s = ['open', 'Open', 'closed', 'Closed', 'all', 'All'];
is that it's not immediately clear which values to localise.

Probably the best approach is the one I used in email.html.tmpl:
http://lxr.mozilla.org/mozilla/source/webtools/bugzilla/template/en/default/account/prefs/email.html.tmpl

Here, that would be:

[% events = [
     { id = 1,
       name = 'open',
       description = "Open" },
     { id = 2,
       name = 'closed',
       description = "Closed" },
     ... 
   ]
%]

I *believe* TT has built-in methods for sorting a hash based on the value of a
given key, which would do the trick nicely.

Gerv

Comment 6

[Emmanuel Seyman]

Attachment: Keep the array and use the has only for i18n purposes

If this doesn't pass review, I'll try Gerv's way.

Comment 7

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 181220 [details] [diff] [review]
Keep the array and use the has only for i18n purposes

This works, but Gerv's approach (minus the id key, which doesn't apply here) is
the better one and more consistent with how this is done elsewhere in the
codebase.  Please submit a patch that takes the approach Gerv has taken, and
I'll review it promptly.

Comment 8

[Emmanuel Seyman]

> Gerv's approach (minus the id key, which doesn't apply here) is the better one

Just to make sure I understand things correctly, the id key must be in this
structure because its value determines the order in which the statuses are
displayed. Correct?

Comment 9

[Gervase Markham [:gerv]]

Exactly - although you should probably call it "sortkey" rather than ID, as
that's the terminology we use elsewhere.

See if you can find that TT method (although I might have been imagining it) for
returning a hash sorted in the order of one of its keys. That'll fix you up.

Of course if there isn't one, we could always write one and add it to the stash :-)

Gerv

Comment 10

[Emmanuel Seyman]

> Exactly - although you should probably call it "sortkey" rather than ID, as
> that's the terminology we use elsewhere.

After my morning coffee, I realized Myk's right.
We're talking about an array of hashes and the order in which the hashes are
returned is always the same.

Fixing, attaching and asking for review

> See if you can find that TT method (although I might have been imagining it)

I've looked in the TT docs and the O'Reilly book. It doesn't seem to exist.

Comment 11

[Emmanuel Seyman]

Attachment: array of hashes, bugzilla-style

Comment 12

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 181368 [details] [diff] [review]
array of hashes, bugzilla-style

>+        [% statuses = [
>+          { status = 'open',
>+            description = "Open" },
>+          { status = 'closed',
>+            description = "Closed" },
>+          { status = 'all',
>+            description = "All" } ]
>+        %]

Nit: this would be more compact and easier to read as:

	[% statuses = [ { status = 'open',   description = "Open" },
			{ status = 'closed', description = "Closed" },
			{ status = 'all',    description = "All" } ] %]

Nit: "status" might be better labeled "name", since each hash as a whole
represents a "status", not just the unique string we use in form field names,
and calling this attribute "status" conflates these values with the hash as a
whole.	Also, while "description" might be better as "label", the industry
standard term for such values.


>+        [% FOREACH s = [ 0 .. statuses.max ] %]
>+            <option value="__[% statuses.${s}.status FILTER html %]__" 
>                   [% " selected" IF default.bug_status.0 == "__${s}__" %]>
>-            [% s %]
>+            [% statuses.${s}.description FILTER html %]

${s} is a number, while default.bug_status.0 is a string like "__open__" or
"__closed__", so those values will never match, and " selected" will never get
generated.

Nit: this would be simpler as:

[% FOREACH s = statuses %]
  <option value="__[% s.status FILTER html %]__" 
    [% " selected" IF default.bug_status.0 == "__${s.status}__" %]>
  [% s.description FILTER html %]
[% END %]

For added readability over the current code, you might switch to "status"
rather than "s".  Also, you don't need to filter these values, since they don't
contain special characters requiring such filtration.  If you're filtering
because otherwise the tests complain, add the applicable variable names to
filterexceptions.pl instead.

Comment 13

[Emmanuel Seyman]

> ${s} is a number, while default.bug_status.0 is a string like "__open__" or
> "__closed__", so those values will never match, and " selected" will never get
> generated.

You're right. My apologies, this is a stupid mistake on my part.

> Also, you don't need to filter these values, since they don't
> contain special characters requiring such filtration.

The new patch doesn't filter label ("status" in the previous patch) but I'm
unsure as to whether I should filter label ("description" in the previous patch)
since we have no idea what these will be translated to in the different
templates. Is this the correct thing to do?

Comment 14

[Emmanuel Seyman]

Attachment: taking into account Myk's comments

Comment 15

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 181518 [details] [diff] [review]
taking into account Myk's comments

You probably don't need to filter label, but it doesn't harm much to do so, and
it's arguably safer.  Great fix!  r=myk

Comment 16

[Myk Melez [:myk] [@mykmelez]]

Approved for checkin during 2.20 freeze.

Comment 17

[Frédéric Buclin]

Checking in template/en/default/filterexceptions.pl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/filterexceptions.pl,v 
<--  filterexceptions.pl
new revision: 1.42; previous revision: 1.41
done
Checking in template/en/default/search/search-specific.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/search/search-specific.html.tmpl,v
 <--  search-specific.html.tmpl
new revision: 1.12; previous revision: 1.11
done