54396 - Hard to delete characters in an autocomplete widget

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect, P2)

Description

[Jean-Francois Ducarroz]

This bug has been extracted from bug 51910 in order to have one fix per bug 
report. The original bug is rtm+/P2.

Either, in the URL location bar or in the Addressing widget in message compose, 
it's hard to delete characters while typing because autocomplete always come 
back and modified the input.

1) Using the addressing widget, type abcdefg
2) wait 300 ms, the input become: abcdef[@netscape.com]
3) press backspace. the input become again abcdef
4) press again backspace to delete the last character. But after 300 ms, it 
become abcde[@netscape.com]

You need to press to much backspace for deleting just one characters!

The fix is to still update the dropdown list with the autocomplete results when 
you backspace (or delete) but we should not autocomplete into the text field 
anymore.

Comment 1

[Jean-Francois Ducarroz]

I have a safe fix for that

Comment 2

[sairuh (rarely reading bugmail)]

jrgm/claudius, who should take this for qa?

Comment 3

[selmer (gone)]

JF, who would hit this and how often?  None of us have ever noticed this problem 
(we all care about 51910 though :-)

Comment 4

[Jean-Francois Ducarroz]

everybody that does typing error and want to correct it. Maybe error while 
typing URL are more frequent. In 4.x, we never autocomplete again an url when 
you hit backspace. But we do in Mozilla because mail use to do it in 4.x.

Again the fix is very safe and everybody will be happy :-)

Comment 5

[Ben Bucksch (:BenB)]

Selmer, after ducarroz's description I saw this bug as well. It happens whenever
you try to correct an URL at a point != the end.

Comment 6

[selmer (gone)]

OK, I believe that's some level of a problem.  I'm not yet convinced it's an
rtm+ kind of problem.  At worst, this seems to be annoying.  I don't think
anyone would give up on the app over it.

Also, I see "Fix in hand" in the status whiteboard, but there is no patch
attached to the bug.  Please attach the patch when you make this notation.

Who has super reviewed the patch and evaluated the risk?

Comment 7

[Ben Bucksch (:BenB)]

Actually, it also happens for the end of the URL. Selmer, try the following:
Enter "home" (will propably be completed to "home.netscape...". Try to delete
everything via backspace. You hit backspace once to delete the selection. One
more time to delete "e". But, after that, it gets completed again. YOu have to
hit backspace again to get "hom". I.e. you end up hitting backspace twice as
much. But even this vaires, depending on the question how fast you as in
comparison to autocomplete. So, yes it's annoying. Show-stopper? No. But we have
a (simple, as ducarroz says) fix.

In any case, please check it into the trunk.

Comment 8

[Jean-Francois Ducarroz]

Attachment: Proposed fix

Comment 9

[selmer (gone)]

rtm+, to give this a chance at PDT approval.  I'm hoping that the super review
says "0 risk" and people running with the patch say "works like a charm".

Comment 10

[Jean-Francois Ducarroz]

The fix is to check if the last key pressed is either a backspace or a
delete. If it's the case, I will force the default result item receive
from the search engine to -1. Therefore, as we don't have a default
item, the content of the text field is not modified by the
autocompletion but we still have the drop down list with all the
potential matches that appears. I have also fixes some annoying
Javascript Strict warning with the properties noDirectMatch & menuOpen
by declaring a "constructor" for those dynamic properties.

The only potential risk is to not autocomplete in line (in the textfield
itself) if for some reason some legitimate keys will generate the same
keycode than backspace or Delete. This case should not append with
Japanese Input as if IME is active, we will get the oninput message
after the onkeypress. I always reset the lastKeyCode to 0 when I receive
an onInput.

I am running this fix since yesterday on Mac & Windows and so far, it's working 
perfectly...

Comment 11

[Ben Bucksch (:BenB)]

"works like a charm" ;)

Comment 12

[Jean-Francois Ducarroz]

Ben, can I put you as reviewer (not the super reviewer)?

Comment 13

[Ben Bucksch (:BenB)]

ducarroz, no, sorry, I don't know the code around it. You can you say "BenB said
it works" :).

Comment 14

[Jean-Francois Ducarroz]

the problem is that I am yhe only one that know this code :-(

Comment 15

[Ben Bucksch (:BenB)]

hyatt? Didn't he help writing autocomplete?

Really, I don't even have a clue about xbl.

Comment 16

[Jean-Francois Ducarroz]

don't worry, I'll find somebody else...

Comment 17

[David :Bienvenu]

well, FWIW, r=bienvenu, with the understanding that alecf's super review would
catch anything really wrong.

Comment 18

[Jean-Francois Ducarroz]

Seth reviewed it too. R=sspitzer

Comment 19

[Alec Flett]

fix looks good
only comment is that there is a way to get to VK_DELETE from JS - you can say
KeyEvent.DOM_VK_DELETE - the same is true for the other VK_ you're using.

that way you don't have to hardcode the numbers

Comment 20

[Jean-Francois Ducarroz]

oh great, much better. I'll do that in the trunk only, don't want to take any 
change to do a typo in the branch :-)... Thanks.

Comment 21

[Jean-Francois Ducarroz]

Alec, I cannot use those DOM_VK_nnn for the following reasons:
1) They doesn't seem to be available from JS
2) you need a even object which I don't have in this case as we are inside a callback from the search engine which 
doesn't relay on events.

Can I get your super review?

Comment 22

[Phil Peterson]

PDT agrees [rtm need info] until super review is available for latest fix.

Comment 23

[Alec Flett]

no, "KeyEvent" is actually a global object of sorts, so you literally say 
something like

if (me.lastKeyPressed == KeyEvent.DOM_VK_DELETE) {
   etc....
}

Comment 24

[Jean-Francois Ducarroz]

I tried that too but it wasn't recognized. Also, I've looked in lxr to see if 
any other JS file use the pattern DOM_VK_ without success!

Comment 25

[Jean-Francois Ducarroz]

I spoke with Alec and we decided to go with the original fix. sr=alecf

Comment 26

[Phil Peterson]

PDT marking [rtm++]

Comment 27

[Jean-Francois Ducarroz]

Fixed and checked in

Comment 28

[Suresh]

Ducarroz, I kinda tested this bug and have a question for you.
If I type 'abcdefgh' in the addressing field, after 300 ms it autocompletes with
'[e-mail]'. If i hit backspace it deletes '@netscape.com' and if I
hit backspace again it deletes 'h'. So I need to hit backspace twice to delete
the last one charecter. Is this the way it is supposed to work? Thanks!!

And btw, if I want to delete more charecters in the right afterwards, then one
backspace is enough for each charecter!  Builds Tested: Today's Branch builds on
win NT and Linux.

Comment 29

[Jean-Francois Ducarroz]

Your are right. 2 backspace the first time (one for the selection and one for 
the last char.) and then only one per characters. Previously you had to hit 
backspace twice for every characters if you were slow!

Comment 30

[Suresh]

So...this works fine for the compose window autocomplete!.

Comment 31

[esther]

Claudius, this still needs to be tested and verified for Browser, Mailnews
checked the Addressing widget for Compose window.

Comment 32

[Claudius Gayle]

VERIFIED Fixed in BRANCH builds 2000101008. Adding vtrunk keywoed so this gets looked at
on the trunk

Comment 33

[Asa Dotzler [:asa]]

Verified Fixed on trunk builds in Browser and Message Compose.  backspace once
removes autocomeplete from actual field and backspace a second time removes last
typed character, after that autocomplete is in the popup not the text widget and
it only takes one backspace per character deleted instead of two.

linux 101808 RedHat 6.2
win32 101804 NT 4
mac 101804 Mac OS9
Setting bug to Verified and removing vtrunk keyword