519142 - Generic display_value should replace status and resolution display IFs

Status: RESOLVED FIXED

(Bugzilla :: User Interface, enhancement)

Description

[Vitaly Fedrushkov]

(In reply to bug 512623 comment #5)
> (In reply to bug 512623 comment #3)
> > As we expect display_value() to expand many fields and do no harm otherwise,
> > IMHO we should stop checking field names whereever possible:

>   We'll do it in a separate bug. I think it's too much to review if all the
> get_status/get_resolution changes aren't just straight automated changes, for
> now.

https://bugzilla.mozilla.org/attachment.cgi?id=396657&action=diff#template/en/default/bug/activity/table.html.tmpl_sec1

| [% ELSIF change.fieldname == 'bug_status' %]
|   [% display_value("bug_status", change.removed) FILTER html %]
| [% ELSIF change.fieldname == 'resolution' %]
|   [% display_value("resolution", change.removed) FILTER html %]

this should read

 [% ELSE %]
   [% display_value(change.fieldname, change.removed) FILTER html %]

and so on

Comment 1

[Vitaly Fedrushkov]

Attachment: patch v1

OK here we start.

This patch addresses some places missed by search-and-replace at attachment 403711 [details] [diff] [review].

Also simple debug feature added: display_value() hits are shown in quotes, and misses in dashes.  This way if you see some value in plain text, then template probably lacks display_value() call.

Comment 2

[Vitaly Fedrushkov]

Caveat emptor: 

This patch does not address admin/fieldvalues interface.  This would require changes to admin/table interface.

Quick search is a problem, too.

Comment 3

[Vitaly Fedrushkov]

Attachment: Patch v2

Unrotted

In debug, context is shown after value, to make bug lists readable.

Comment 4

[Max Kanat-Alexander]

Comment on attachment 412693 [details] [diff] [review]
Patch v2

This looks great, except for the "debug" change to field-descs.none.tmpl, which I'd prefer to see in another bug (as I'd have to more carefully check how that impacts various pages).

Comment 5

[Max Kanat-Alexander]

So this can be checked in, except for the field-descs "debug" change.

Comment 6

[Vitaly Fedrushkov]

Attachment: Patch v3

Debug code omitted

Comment 7

[Vitaly Fedrushkov]

Attachment: Debug piece

(In reply to comment #4)
> (From update of attachment 412693 [details] [diff] [review])
> This looks great, except for the "debug" change to field-descs.none.tmpl, which
> I'd prefer to see in another bug (as I'd have to more carefully check how that
> impacts various pages).

This patch should not be committed.  Instead it can be used by localizers and Bugzilla instance maintainers to troubleshoot field-descs.none.tmpl.

PLEASE NOTE: this patch may severely hurt Bugzilla performance, especially bug lists.

Comment 8

[Max Kanat-Alexander]

Thanks for this patch, Vitaly! That was really a monster! :-)

Checking in template/en/default/admin/params/common.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/params/common.html.tmpl,v  <--  common.html.tmpl
new revision: 1.8; previous revision: 1.7
done
Checking in template/en/default/admin/workflow/comment.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/workflow/comment.html.tmpl,v  <--  comment.html.tmpl
new revision: 1.5; previous revision: 1.4
done
Checking in template/en/default/admin/workflow/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/workflow/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.8; previous revision: 1.7
done
Checking in template/en/default/bug/show-multiple.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/show-multiple.html.tmpl,v  <--  show-multiple.html.tmpl
new revision: 1.48; previous revision: 1.47
done
Checking in template/en/default/bug/activity/table.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/activity/table.html.tmpl,v  <--  table.html.tmpl
new revision: 1.20; previous revision: 1.19
done
Checking in template/en/default/bug/create/create.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/create/create.html.tmpl,v  <--  create.html.tmpl
new revision: 1.99; previous revision: 1.98
done
Checking in template/en/default/global/code-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/code-error.html.tmpl,v  <--  code-error.html.tmpl
new revision: 1.121; previous revision: 1.120
done
Checking in template/en/default/global/field-descs.none.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/field-descs.none.tmpl,v  <--  field-descs.none.tmpl
new revision: 1.38; previous revision: 1.37
done
Checking in template/en/default/list/edit-multiple.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/list/edit-multiple.html.tmpl,v  <--  edit-multiple.html.tmpl
new revision: 1.60; previous revision: 1.59
done
Checking in template/en/default/list/list.atom.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/list/list.atom.tmpl,v  <--  list.atom.tmpl
new revision: 1.7; previous revision: 1.6
done
Checking in template/en/default/list/list.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/list/list.html.tmpl,v  <--  list.html.tmpl
new revision: 1.71; previous revision: 1.70
done
Checking in template/en/default/list/table.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/list/table.html.tmpl,v  <--  table.html.tmpl
new revision: 1.51; previous revision: 1.50
done
Checking in template/en/default/pages/bug-writing.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/pages/bug-writing.html.tmpl,v  <--  bug-writing.html.tmpl
new revision: 1.10; previous revision: 1.9
done
Checking in template/en/default/pages/fields.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/pages/fields.html.tmpl,v  <--  fields.html.tmpl
new revision: 1.19; previous revision: 1.18
done
Checking in template/en/default/reports/duplicates-table.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/duplicates-table.html.tmpl,v  <--  duplicates-table.html.tmpl
new revision: 1.16; previous revision: 1.15
done
Checking in template/en/default/reports/report-bar.png.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/report-bar.png.tmpl,v  <--  report-bar.png.tmpl
new revision: 1.10; previous revision: 1.9
done
Checking in template/en/default/reports/report-line.png.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/report-line.png.tmpl,v  <--  report-line.png.tmpl
new revision: 1.11; previous revision: 1.10
done
Checking in template/en/default/reports/report-pie.png.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/report-pie.png.tmpl,v  <--  report-pie.png.tmpl
new revision: 1.9; previous revision: 1.8
done
Checking in template/en/default/reports/report-table.csv.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/report-table.csv.tmpl,v  <--  report-table.csv.tmpl
new revision: 1.14; previous revision: 1.13
done
Checking in template/en/default/reports/report-table.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/report-table.html.tmpl,v  <--  report-table.html.tmpl
new revision: 1.19; previous revision: 1.18
done
Checking in template/en/default/search/form.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/search/form.html.tmpl,v  <--  form.html.tmpl
new revision: 1.63; previous revision: 1.62
done
Checking in template/en/default/whine/mail.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/whine/mail.html.tmpl,v  <--  mail.html.tmpl
new revision: 1.9; previous revision: 1.8
done
Checking in template/en/default/whine/mail.txt.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/whine/mail.txt.tmpl,v  <--  mail.txt.tmpl
new revision: 1.9; previous revision: 1.8
done

Comment 9

[A. Shimono [:himorin]]

Comment on attachment 412706 [details] [diff] [review]
Patch v3

>Index: global/code-error.html.tmpl
>===================================================================
>RCS file: /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/code-error.html.tmpl,v

>   [% ELSIF error == "no_manual_moved" %]
>-    You cannot set the resolution of [% terms.abug %] to MOVED without
>+    You cannot set the resolution of [% terms.abug %] to display_value("resolution", "MOVED") FILTER html %] without
>     moving the [% terms.bug %].

We definitely need an open blacket here.
>+    You cannot set the resolution of [% terms.abug %] to [% display_value("resolution", "MOVED") FILTER html %] without

I'd like to propose r-, but too late. :-p