740879 - If you're not allowed to change status or resolution, their values are being displayed unlocalized

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[Marc Schumann [:Wurblzap]]

Attachment: Patch

If you're not allowed to change status or resolution, their values are being displayed unlocalized below the comment textarea.

Comment 1

[Frédéric Buclin]

4.0 and older are restricted to security bugs only.

Comment 2

[Frédéric Buclin]

Comment on attachment 610930 [details] [diff] [review]
Patch

> [% ELSE %]
>+  [% FOREACH val = value %]
>+    [% display_value(field.name, val) FILTER html %]
>+    [% ', ' UNLESS loop.last() %]
>+  [% END %]
> [% END %]

Hum, I think you should check field.type and only call display_value() for single-select and multi-select fields. Other fields do not need to call display_value(). This would avoid useless calls to display_value() and would be consistent with the editable part of the template.

Comment 3

[Marc Schumann [:Wurblzap]]

Attachment: Patch 1.1

I'm not really in agreement, because it's pure chance that display_value does not need to be called for other field types. For maintainability, we should call display_value no matter what.

In order to get in a fix, here's a modified patch anyway.

Comment 4

[Frédéric Buclin]

Comment on attachment 611227 [details] [diff] [review]
Patch 1.1

It's not pure chance. Select fields are the only ones with a defined list of values. Why would you localize e.g. a free text field? This doesn't really make sense. Anyway, could you please use ELSIF instead of SWITCH to remain consistent with the code above your code? IF ELSIF ELSIF ELSE SWITCH doesn't make sense.

Comment 5

[Marc Schumann [:Wurblzap]]

Attachment: Patch 1.2

All right, it doesn't make sense for all field types.

Here's an updated patch. For consistency with the [% IF editable %] part, I moved to SWITCH/CASE instead of ELSIF.

Comment 6

[Frédéric Buclin]

Comment on attachment 611230 [details] [diff] [review]
Patch 1.2

Works fine, nice cleanup. :) r=LpSolit

Comment 7

[Marc Schumann [:Wurblzap]]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified template/en/default/bug/field.html.tmpl
Committed revision 8181.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.2/
modified template/en/default/bug/field.html.tmpl
Committed revision 8067.