Closed Bug 357315 Opened 18 years ago Closed 17 years ago

Ability to create <textarea> fields

Categories

(Bugzilla :: Creating/Changing Bugs, enhancement)

2.23.3
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 3.2

People

(Reporter: mkanat, Assigned: mkanat)

References

Details

(Whiteboard: [roadmap: 3.2])

Attachments

(1 file, 2 obsolete files)

We already have plain-text <input> fields--it should be very easy to create <textarea> fields. In fact, I wrote the patch just now--it took only a few minutes.
Attached patch v1 (obsolete) (deleted) — Splinter Review
Okay, here we go. I haven't tested this extensively, but I did do some basic tests and it seems to work just fine.
Assignee: create-and-change → mkanat
Status: NEW → ASSIGNED
Attachment #242798 - Flags: review?(bugzilla-mozilla)
Comment on attachment 242798 [details] [diff] [review] v1 >Index: template/en/default/bug/field.html.tmpl >+ [% CASE constants.FIELD_TYPE_TEXTAREA %] >+ <textarea name="[% field.name FILTER html%]" >+ cols="60" rows="4">[% value FILTER html %]</textarea> > [% END %] We really have to group fields by type. If someone has 3 text boxes, 2 dropdown menus, and 3 textareas, the UI will be very very ugly. But that's another bug.
It is not really a problem due to the sortkey on custom fields.
Yeah, right. But what I meant is that we may want to display them in different places in the page based on their type.
Comment on attachment 242798 [details] [diff] [review] v1 >Index: template/en/default/bug/field.html.tmpl >@@ -56,6 +56,9 @@ > [%- legal_value FILTER html %]</option> > [% END %] > </select> >+ [% CASE constants.FIELD_TYPE_TEXTAREA %] >+ <textarea name="[% field.name FILTER html%]" >+ cols="60" rows="4">[% value FILTER html %]</textarea> Nit: you could use global/textarea.html.tmpl review-: When editable is set to '0', you should use FILTER html_linebreak, not FILTER html. Nit: When changing bugs, it doesn't show spaces at the beginning of lines or empty lines. However, bugmail might to this intentionally.
Attachment #242798 - Flags: review?(bugzilla-mozilla) → review-
The trunk is now frozen -> TM: 3.2
Target Milestone: Bugzilla 3.0 → Bugzilla 3.2
Whiteboard: [roadmap: 3.2]
Attached patch v2 (obsolete) (deleted) — Splinter Review
Whoever gets to this first is fine with me. This patch is sponsored by NASA, so reviews will also be sponsored by NASA.
Attachment #242798 - Attachment is obsolete: true
Attachment #280828 - Flags: review?(bugzilla-mozilla)
Attachment #280828 - Flags: review?(LpSolit)
Comment on attachment 280828 [details] [diff] [review] v2 >Index: Bugzilla/Field.pm >+ FIELD_TYPE_TEXTAREA, { TYPE => 'MEDIUMTEXT' }, Nit: when you edit this field (which may contain e.g. a whole big comment or a stack trace), this huge field appears in the bugs activity table and in bugmail. I let you imagine how unreadable it becomes. Do you plan to do something to prevent this? >Index: template/en/default/bug/field.html.tmpl >+ [% INCLUDE global/textarea.html.tmpl >+ id = field.name name = field.name minrows = 4 maxrows = 8 >+ cols = 65 defaultcontent = value %] Set cols = 60 for consistency with free text fields and other free text fields in show_bug. >- [% value.join(', ') FILTER html %] >+ [% value.join(', ') FILTER html_linebreak %] html_linebreak doesn't filter anything. You can inject XSS now (and 008filter.t correctly complains about it). You must write FILTER html FILTER html_linebreak. Anyway, from what I understand, html_linebreak must only be used in element attributes, e.g. <foo bar="[% baz FILTER html_linebreak FILTER html %]", which is not the case here. So you should leave this filter alone. > ${constants.FIELD_TYPE_FREETEXT} => "Free Text", > ${constants.FIELD_TYPE_SINGLE_SELECT} => "Drop Down", > ${constants.FIELD_TYPE_MULTI_SELECT} => "Multiple-Selection Box", >+ ${constants.FIELD_TYPE_TEXTAREA} => "Large Text Box", I note that the UI displays these fields in some random order. We should really fix that. Bug that's another bug. Else your patch looks good.
Attachment #280828 - Flags: review?(bugzilla-mozilla)
Attachment #280828 - Flags: review?(LpSolit)
Attachment #280828 - Flags: review-
Also, maybe should we enclose the field in <pre></pre> when you cannot edit the field so that newlines and indentation are respected.
I would vote against using PRE, since it essentially overrides the standard CSS on the page.. rendering your text in a fixed width font. Why would html_linebreak only be used in attribute definitions? Also [% baz FILTER html_line_break FILTER html %] since the <br/> that would be injected would be escaped so that they would be printed to the screen incorrectlyas &lt;br /&gt; &lt;br /&gt;. I see no reason why you wouldn't want to FILTER html then insert BR's instead of \n when you are not in a text area.
(In reply to comment #10) > I would vote against using PRE, since it essentially overrides the standard CSS > on the page.. rendering your text in a fixed width font. That's how bug comments are displayed already. And you can still override PRE in CSS files. > Why would html_linebreak only be used in attribute definitions? Also [% baz > FILTER html_line_break FILTER html %] since the <br/> that would be injected > would be escaped so that they would be printed to the screen incorrectlyas > &lt;br /&gt; &lt;br /&gt;. Huh? Did you read the definition of html_linebreak at http://mxr.mozilla.org/mozilla/source/webtools/bugzilla/Bugzilla/Template.pm#488? No <br> is injected and no newline appears in the HTML page.
We can use CSS with font-style: monospace and white-space: pre.
Ahh, that would explain why you used html_linebreak instead of html_line_break which is provided by the template toolkit library its self. Based on the definition html_linebreak is a terriable name, and as you indicated likely not useful in this situation.
Attached patch v3 (deleted) — Splinter Review
This version uses a div with white-space: pre and font-family: monospace. I also made wrap_comment accept an argument, and fixed a tiny bug in wrap_comment.
Attachment #280828 - Attachment is obsolete: true
Attachment #280938 - Flags: review?(LpSolit)
Oh, and I forgot to change "65" to "60", but I can do that on checkin.
Comment on attachment 280938 [details] [diff] [review] v3 >Index: skins/standard/global.css >+/* For bug fields */ >+.uneditable_textarea { Nit: this class should have a more generic name as it could be reused in some other places too. >Index: template/en/default/bug/field.html.tmpl >+ [% INCLUDE global/textarea.html.tmpl >+ id = field.name name = field.name minrows = 4 maxrows = 8 >+ cols = 65 defaultcontent = value %] > [% END %] >+[% ELSIF field.type == constants.FIELD_TYPE_TEXTAREA %] >+ <div class="uneditable_textarea">[% value FILTER wrap_comment(65) >+ FILTER html %]</div> Do not forget to s/65/60/ in both places on checkin. r=LpSolit
Attachment #280938 - Flags: review?(LpSolit) → review+
Flags: approval+
I didn't change the class name. If anybody wants to re-use the same style on something else unrelated, they're free to just add another class name to the same format description. (Like .other_class, .uneditable_textarea { }). Checking in Bugzilla/Constants.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Constants.pm,v <-- Constants.pm new revision: 1.78; previous revision: 1.77 done Checking in Bugzilla/Field.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Field.pm,v <-- Field.pm new revision: 1.28; previous revision: 1.27 done Checking in Bugzilla/Template.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Template.pm,v <-- Template.pm new revision: 1.79; previous revision: 1.78 done Checking in Bugzilla/Util.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Util.pm,v <-- Util.pm new revision: 1.60; previous revision: 1.59 done Checking in skins/standard/global.css; /cvsroot/mozilla/webtools/bugzilla/skins/standard/global.css,v <-- global.css new revision: 1.40; previous revision: 1.39 done Checking in template/en/default/bug/field.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/field.html.tmpl,v <-- field.html.tmpl new revision: 1.11; previous revision: 1.10 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.24; previous revision: 1.23 done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Blocks: 99240
Blocks: 434008
No longer blocks: 434008
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: