253014 - commit a new bug or comment with a shortcut (control-enter) in the comment box.

Status: RESOLVED WONTFIX

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[Tiago Mello [:timello]]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040626 Firefox/0.9.1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040626 Firefox/0.9.1

It's simple, but I think it be very useful.

Reproducible: Always
Steps to Reproduce:
1.
2.
3.

Comment 1

[Tiago Mello [:timello]]

Attachment: Here it's. It solve the bug with a javascript event (onKeyDown).

Comment 2

[Christian Reis]

Comment on attachment 154313 [details] [diff] [review]
Here it's. It solve the bug with a javascript event (onKeyDown).

>Index: js/form_utils.js
>+function shortcut_commit(event, form) {
>+  if (event.ctrlKey && event.keyCode == 13) {
>+    form.submit();
>+    return false;
>+  }
>+}

Maybe call the function:

  submit_on_ctrl_enter() 

to make it very clear?

>Index: template/en/default/bug/process/header.html.tmpl
>-[% PROCESS global/header.html.tmpl %]
>+[% PROCESS global/header.html.tmpl 
>+   javascript_urls = [ "/js/form_utils.js" ]
>+%]

I don't see why we need this in process.. there's no form with a textarea there
being altered, is there?

Comment 3

[Tiago Mello [:timello]]

(In reply to comment #2)
> >Index: template/en/default/bug/process/header.html.tmpl
> >-[% PROCESS global/header.html.tmpl %]
> >+[% PROCESS global/header.html.tmpl 
> >+   javascript_urls = [ "/js/form_utils.js" ]
> >+%]
> 
> I don't see why we need this in process.. there's no form with a textarea there
> being altered, is there?

Humm, we have 3 situation where we want to put this feature: 
 1 - When creating a new bug (enter_bug.cgi);
 2 - After bug created (post_bug.cgi);
 3 - When commenting an existing bug (show_bug.cgi);
However, post_bug.cgi don't use the same header template. It use
results.html.tmpl which call process/header.html.tmpl, so we need this.
Am I wrong?
Perhaps, Can be better to add interfaces in process/header.html.tmpl?

Comment 4

[Christian Reis]

Ah, I see -- post_bug uses process/header.html.tmpl. Yeah, leave that in.

Comment 5

[Tiago Mello [:timello]]

Attachment: Patch for submit on control enter

Kiko forgives me!. You are certain. We don't need to change the
process/header.html.tmpl.

Comment 6

[Christian Reis]

Comment on attachment 154613 [details] [diff] [review]
Patch for submit on control enter

Looks good to me. This feature is quite non-intrusive, and though not easily
detectable by the end-user it can be quite a time-saver for advanced users. 

I think we should document this in the guide as well -- what do you think, Myk?

Comment 7

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 154613 [details] [diff] [review]
Patch for submit on control enter

I'm a little concerned about users triggering submits inadvertently, since
ctrl-enter already does things for links in the page and the location bar (in
Mozilla-based browsers, anyway).  Also, "tab enter" is already a shortcut for
doing this, albeit unofficially.  How would this be better?

Comment 8

[Christian Reis]

(In reply to comment #7)
> (From update of attachment 154613 [details] [diff] [review])
> I'm a little concerned about users triggering submits inadvertently, since
> ctrl-enter already does things for links in the page and the location bar (in
> Mozilla-based browsers, anyway). 

Well, what it does (opening the link itself, albeit in a new tab) seems almost
consistent with what this patch does.

> Also, "tab enter" is already a shortcut for
> doing this, albeit unofficially.  How would this be better?

It would offer a direct way to commit from the comment box -- it could even be
documented or a Bugzilla-wide idiom.

I'm not too keen on this change myself, but I don't think it *harms* us much.
Perhaps I should ask on the developer list what people think, but they tend to
be such conservatives :-)

Comment 9

[Myk Melez [:myk] [@mykmelez]]

> Well, what it does (opening the link itself, albeit in a new tab) seems almost
> consistent with what this patch does.

The difference is that submitting the form irreversibly writes the comment to
the bug.  For a user halfway through writing a comment, this is frustrating and
potentially embarrassing.

In Thunderbird, ctrl-enter in a compose window sends the message, but it prompts
the user first (with an option for turning off prompting).

> It would offer a direct way to commit from the comment box -- it could even be
> documented or a Bugzilla-wide idiom.

"tab enter" is just as direct.  ctrl-enter is more standard, and it has the
advantage of being independent of any UI configuration, so it's more
future-proof, but this can be mitigated in the "tab enter" case by documenting
the behavior and preserving it in future UI changes.

> I'm not too keen on this change myself, but I don't think it *harms* us much.
> Perhaps I should ask on the developer list what people think, but they tend to
> be such conservatives :-)

There's some potential harm and not much advantage over the current shortcut. 
We can mitigate the harm with a Thunderbird-like pref, but that's significantly
more work.  Unless and until we do that work, I think we're better off with the
current shortcut.

Comment 10

[Christian Reis]

(In reply to comment #9)
> > Well, what it does (opening the link itself, albeit in a new tab) seems almost
> > consistent with what this patch does.
> 
> The difference is that submitting the form irreversibly writes the comment to
> the bug.  For a user halfway through writing a comment, this is frustrating and
> potentially embarrassing.

I meant that control-enter on *any other* form control apart from the Comment
box submits the bug. So in a way we're just making it more consistent -- it's
not that control-enter anywhere else is safer. Were you aware of this?

You might be raising the point that control-enter is more possible to be hit
accidentally while editing text, but I think that's rather pushing it -- I
*never* did so unintentionally (as it's not an easy to reach chord). 

Comment 11

[Myk Melez [:myk] [@mykmelez]]

> I meant that control-enter on *any other* form control apart from the Comment
> box submits the bug. So in a way we're just making it more consistent -- it's
> not that control-enter anywhere else is safer. Were you aware of this?

Yes, although it isn't control key-specific; hitting just "enter" on many form
controls submits forms in common browsers.  This is the behavior that the "tab
enter" shortcut relies upon.  Perhaps this should be more consistent, but enter
has special meaning in textareas, and making it more consistent would be a
browser bug anyway.

> You might be raising the point that control-enter is more possible to be hit
> accidentally while editing text, but I think that's rather pushing it -- I
> *never* did so unintentionally (as it's not an easy to reach chord). 

I've done it before to my chagrin.

Comment 12

[Christian Reis]

(In reply to comment #11)
> > You might be raising the point that control-enter is more possible to be hit
> > accidentally while editing text, but I think that's rather pushing it -- I
> > *never* did so unintentionally (as it's not an easy to reach chord). 
> 
> I've done it before to my chagrin.

While editing a textarea? How could you tell if currently it does nothing? Do
you mean on something else besides Bugzilla?

Comment 13

[Myk Melez [:myk] [@mykmelez]]

>Do you mean on something else besides Bugzilla?

Yes, I mean in my email client.

Comment 14

[Vlad Dascalu]

Comment on attachment 154613 [details] [diff] [review]
Patch for submit on control enter

Since you're not borrowing (copy-pasting) code from other files when you're
creating a new one, you should respect the mozilla.org policy:

http://www.mozilla.org/MPL/relicensing-faq.html#new-license

and use the tri-licensing header.

Comment 15

[Gervase Markham [:gerv]]

That policy does not apply to Bugzilla, which remains MPL-only. That document
says "new Mozilla source files"; while slightly behind the times, the intent is
clear. The "webtools" directory is not listed in the next section.

Gerv

Comment 16

[Vlad Dascalu]

Comment on attachment 154613 [details] [diff] [review]
Patch for submit on control enter

Changing review- to review+ after reading Gerv's comments and reviewing again
the patch.

Comment 17

[Christian Reis]

Well, the code works, and the feature is useful, so someone needs to make
decision of whether it should apply to CVS HEAD or not. 

If it was my choice, I would apply it to HEAD, leave it in for a while, and if
it showed itself a problem, roll it back -- that's one of the advantages of
having version control in place, right?

Comment 18

[Dave Miller [:justdave]]

I'm kind of thinking this needs an alert box to warn you it's about to submit if
we do this...  on my keyboard at least (it's a laptop), the control key (and a
few others) are pretty easy to hit on accident.

Comment 19

[Myk Melez [:myk] [@mykmelez]]

> If it was my choice, I would apply it to HEAD, leave it in for a while, and if
> it showed itself a problem, roll it back -- that's one of the advantages of
> having version control in place, right?

Yes, in fact revertability is the principle advantage of version control, but in
this case I don't think this patch is the right change for the reasons mentioned
previously (availability of alternative and danger of accidental use).

In theory it would be possible to create a version of this patch that prompted
the user before submitting a comment and included an option for turning off
prompting on a per-user basis (as ctrl-enter works in Thunderbird now for
sending messages), but that's way to much effort and additional Bugzilla
complexity (especially given the existing alternative) and something more
appropriately implemented in browsers for all textareas.