768573 - bug/field.html.tmpl overrides the 'hidden' variable, causing some field labels to be hidden by accident

Status: RESOLVED FIXED

(Bugzilla :: User Interface, defect)

Description

[Frédéric Buclin]

While reviewing bug 766528, I realized that several labels were missing in the UI. The reason was that the last custom field was hidden and so the hidden variable was set to 1. As bug/field.html.tmpl is PROCESS'ed, this variable is visible outside this template, and affects all other template calls, for instance bug/field-label.html.tmpl. To force the variable to remain local, the bug/field.html.tmpl template must be INCLUDE'd, not PROCESS'ed.

Comment 1

[Frédéric Buclin]

Attachment: patch, v1

Comment 2

[:glob ✱]

Comment on attachment 636807 [details] [diff] [review]
patch, v1

r=glob

Comment 3

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified template/en/default/bug/edit.html.tmpl
modified template/en/default/bug/knob.html.tmpl
modified template/en/default/bug/show-multiple.html.tmpl
modified template/en/default/list/edit-multiple.html.tmpl
Committed revision 8271.


Committing to: bzr+ssh://[e-mail]/bugzilla/4.2/
modified template/en/default/bug/edit.html.tmpl
modified template/en/default/bug/knob.html.tmpl
modified template/en/default/bug/show-multiple.html.tmpl
modified template/en/default/list/edit-multiple.html.tmpl
Committed revision 8098.

Comment 4

[Frédéric Buclin]

The patch causes test_custom_fields.t to fail. The reason is that extra_field_item is no longer accessible outside bug/field.html.tmpl, and so bug/edit.html.tmpl can no longer use it to display related bugs. I suspect bug/field.html.tmpl should display the extra_field_item data itself.

Comment 5

[Frédéric Buclin]

If the fix is not trivial, we should simply backout the patch from the 4.2 branch as it's not necessary there. For trunk, we need it to not regress bug 766528.

Comment 6

[Frédéric Buclin]

Ok, backed out from the 4.2 branch: Committed revision 8105. As I said in comment 5, we still need it for trunk.

Comment 7

[Frédéric Buclin]

I also backed out the patch from the trunk: Committed revision 8309. I have a better fix.

Comment 8

[Frédéric Buclin]

Attachment: patch, v2

This is a much better patch which simply renames the 'hidden' variable to 'field_hidden', which isn't used elsewhere and so won't override the more commonly used 'hidden' variable. Also, INCLUDE'ing bug/field-label.html.tmpl instead of PROCESS'ing it will prevent this template from altering this variable too. This was the single place in our codebase to use PROCESS instead of INCLUDE when calling this template.

Comment 9

[:glob ✱]

Comment on attachment 646912 [details] [diff] [review]
patch, v2

r=glob much cleaner :)

Comment 10

[Frédéric Buclin]

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