873250 - Having enter select autocomplete is annoying.

Status: RESOLVED FIXED

(DevTools :: Console, defect, P2)

Description

[Potch [:potch]]

When I'm debugging things, I tend to have short variable names. I try to interact with these variables in the console, to the following effect:

> var a = complicatedLineOfJavaScript.getComplexValue();
> a[ddEventListener]
(presses enter to see value of 'a')
> addeventListener
function addEventListener() {
    [native code]
}

at this point I get sad.

Comment 1

[Rob Campbell [:rc] (:robcee)]

taking this because we are all saddened by this particular usability nugget.

Comment 2

[Rob Campbell [:rc] (:robcee)]

https://tbpl.mozilla.org/?tree=Try&rev=51c8092033dc

Comment 3

[Rob Campbell [:rc] (:robcee)]

Attachment: enter-key

This does the most basic thing: Enter Key always Evaluates. Now to autocomplete, you have to press a tab key, even with the popup open.

I tried a simple variant where if a user presses up or down arrows, it autocompletes on enter, but it was adding more logic rather than taking away. I think simpler is better.

The one slightly annoying case with this is that you will now have to press three keys to select an autocompletion and evaluate it. So to inspect a variable in scope, you might have to select Up then Tab then Enter.

Comment 4

[Simon Lindholm]

FWIW, Firebug solves this by letting Enter evaluate iff the selected completion is an exact match (http://code.google.com/p/fbug/issues/detail?id=4931). It has worked pretty well for us.

Comment 5

[Mihai Sucan [:msucan]]

(In reply to Rob Campbell [:rc] (:robcee) from comment #3)
> Created attachment 756959 [details] [diff] [review]
> enter-key
> 
> This does the most basic thing: Enter Key always Evaluates. Now to
> autocomplete, you have to press a tab key, even with the popup open.
> 
> I tried a simple variant where if a user presses up or down arrows, it
> autocompletes on enter, but it was adding more logic rather than taking
> away. I think simpler is better.

Agreed. Simple is better here. We should never break user's expectation for what Enter does.


> The one slightly annoying case with this is that you will now have to press
> three keys to select an autocompletion and evaluate it. So to inspect a
> variable in scope, you might have to select Up then Tab then Enter.

I tested this case and it seems fine for me with the patch applied.

Comment 6

[Mihai Sucan [:msucan]]

Comment on attachment 756959 [details] [diff] [review]
enter-key

Review of attachment 756959 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you for the patch! r+ with the comments below addressed.

::: browser/devtools/webconsole/test/browser_webconsole_bug_585991_autocomplete_keys.js
@@ +199,1 @@
>           "completion was successful after VK_RETURN");

Shouldn't this code also check that the string that was executed is not foobar.valueOf?  Please also check that the last history entry is the .value string.

This change alone makes the test confusing.

::: browser/devtools/webconsole/webconsole.js
@@ +3865,4 @@
>          }
>          break;
>  
> +      // Bug 873250 - always enter, ignore autocomplete unless inputUpdated

inputUpdated? This looks like a remnant of some older patch version.

Comment 7

[Rob Campbell [:rc] (:robcee)]

(In reply to Mihai Sucan [:msucan] from comment #6)
> Comment on attachment 756959 [details] [diff] [review]
> enter-key
> 
> Review of attachment 756959 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thank you for the patch! r+ with the comments below addressed.
> 
> :::
> browser/devtools/webconsole/test/
> browser_webconsole_bug_585991_autocomplete_keys.js
> @@ +199,1 @@
> >           "completion was successful after VK_RETURN");
> 
> Shouldn't this code also check that the string that was executed is not
> foobar.valueOf?  Please also check that the last history entry is the .value
> string.

maybe. I was going for simple here. Will update.

> This change alone makes the test confusing.
> 
> ::: browser/devtools/webconsole/webconsole.js
> @@ +3865,4 @@
> >          }
> >          break;
> >  
> > +      // Bug 873250 - always enter, ignore autocomplete unless inputUpdated
> 
> inputUpdated? This looks like a remnant of some older patch version.

yes, forgot to fix that comment before I qrefed.

thanks!

Comment 8

[Rob Campbell [:rc] (:robcee)]

Attachment: enter-key v2

updated the unittest.

Comment 9

[Rob Campbell [:rc] (:robcee)]

https://hg.mozilla.org/integration/fx-team/rev/1095f46b0220

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/1095f46b0220