Bug 26950 – Make the summary and alias fields support click-to-edit

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Maciej Stachowiak]

Make the summary and alias fields support click-to-edit, so Bugzilla can be cool like Flickr.

[Maciej Stachowiak]

Created attachment 32224 [details]
patch

[Maciej Stachowiak]

My patch is completely untested - I could not figure out how to set up a bugzilla test server to try it out.

[Maciej Stachowiak]

Created attachment 32225 [details]
patch

[David Kilzer (:ddkilzer)]

Comment on attachment 32225 [details]
patch

>-function hideEditableField( container, input, action, field_id, original_value ) {
>+function hideEditableField( container, input, action, field1_text, field2_text, field_id, original_value ) {
>     YAHOO.util.Dom.setStyle(container, 'display', 'inline');
>     YAHOO.util.Dom.setStyle(input, 'display', 'none');
>-    YAHOO.util.Event.addListener(action, 'click', showEditableField,
>+    YAHOO.util.Event.addListener(action, 'click', showEditableFieldFocusLast,
>+                                 new Array(container, input));
>+    YAHOO.util.Event.addListener(field2_text, 'click', showEditableFieldFocusLast,
>+                                 new Array(container, input));
>+    YAHOO.util.Event.addListener(field1_text, 'click', showEditableField,
>                                  new Array(container, input));

Since field2_text is field_id with "_nonedit_display" appended and field1_text is "alias_nonedit_display", I don't think you need to add two parameters here.

Also, adding a new function, showEditableFieldFocusLast(), instead of adding arguments to the array object goes against the design of the JavaScript used elsewhere in the file.

>-function showEditableField (e, ContainerInputArray) {
>+function showEditableField (e, ContainerInputArray, focusLast) {

The "focusLast" argument should be passed in with the ContainerInputArray argument.

>     }
>+
>+
>     YAHOO.util.Event.preventDefault(e);
> }

Gratuitous whitespace change.

>+function showEditableFieldFocusLast (e, ContainerInputArray) {
>+    showEditableField(e, ContainerInputArray, true);
>+}

This function won't be needed if you add the "focusLast" argument to the ContainerInputArray.

>     hideEditableField( 'summary_alias_container','summary_alias_input',
>-                       'editme_action','short_desc', short_desc_value);  
>+                       'editme_action', 'alias_nonedit_display', 'short_desc_nonedit_display', 'short_desc', short_desc_value);  

Again, I think we only need to add one argument to hideEditableField().

r- for the above issues.  (Patches to Bugzilla should try to match the existing design to make them easier to merge upstream.)

FWIW, this patch did work using a local copy of the original page (saved as HTML) with a <base> tag added and the URL to the field.js adjusted to use a local copy.

I have an updated patch that I'll post next.

[David Kilzer (:ddkilzer)]

Created attachment 32240 [details]
Patch v3

Patch with issues in Comment #4 addressed.

[David Kilzer (:ddkilzer)]

Filed a Bugzilla bug for this enhancement request:

Bug 502249: Make the short description and alias fields support click-to-edit
<https://bugzilla.mozilla.org/show_bug.cgi?id=502249>

[Maciej Stachowiak]

Comment on attachment 32240 [details]
Patch v3

r=me on your changes. Not sure which one of us should land the patch now. :-)

[David Kilzer (:ddkilzer)]

(In reply to comment #7)
> (From update of attachment 32240 [details])
> r=me on your changes. Not sure which one of us should land the patch now. :-)

I got it.  :)

[David Kilzer (:ddkilzer)]

$ git svn dcommit
Committing to http://svn.webkit.org/repository/webkit/trunk ...
        M       BugsSite/ChangeLog
        M       BugsSite/js/field.js
Committed r45538

http://trac.webkit.org/changeset/45538