When composing a new message, using the keyboard to select a recipient from the auto-complete drop-down, the wrong recipient is filled into the field. This problem first appeared in Daily 20160909 (and continues to e.g. 0913) Daily 0908 does not have this problem. I've marked it "major", due to the likelihood of the email then going to the wrong person, if the user does not spot the wrong fill (which has happened to me). To reproduce: • compose new mail, To: whoever • in next recipient box below, type first 2 letters of a different recipient • auto-complete drop-down appears; box is pre-filled with first entry from drop-down • move down the drop-down list using keyboard cursor-down, to a different recipient (not the first pre-filled entry) • enter/return to select that recipient: drop-down disappears •notice that the wrong, originally pre-filled recipient remains (the first entry in the drop-down), not the one selected via keyboard cursor/enter from the list It seems that if you select from the drop-down using the mouse, it is OK. It is wrong if the drop-down entry is selected via the keyboard.

Does it also happen if you have thunderbird started in safe mode? (is typically good to have this test result in a bug report)

(In reply to Wayne Mery (:wsmwk, NI for questions) from comment #1) > Does it also happen if you have thunderbird started in safe mode? > (is typically good to have this test result in a bug report) Sorry Wayne, should have remembered that. Yes, reproduced with 0913 in safe mode.

Yes, reproducible in Daily of yesterday. Alice, could you find us the regression. Look like some autocomplete change in M-C between 2016-09-08 and 2016-09-09 as per comment #0.

Last Good: http://archive.mozilla.org/pub/thunderbird/tinderbox-builds/comm-central-win32/1473329073/ c-c https://hg.mozilla.org/comm-central/rev/3b028c289baa00abbc8666d9873e3162af6f1bb0 First Bad: http://archive.mozilla.org/pub/thunderbird/tinderbox-builds/comm-central-win32/1473415341/ c-c https://hg.mozilla.org/comm-central/rev/3b028c289baa00abbc8666d9873e3162af6f1bb0 There are no m-c change set information. I cannot find regression range in m-c until fix Bug 1297198

Thanks! Oops, you pasted the same C-C link twice. Can you paste the link to search the M-C pushlog by date, I'll have a look. The regression range appears to be 24 hours only.

(In reply to Jorg K (GMT+2, PTO during summer) from comment #5) > Thanks! > Oops, you pasted the same C-C link twice. good and bad build has same c-c. and no m-c information in the builds. > Can you paste the link to search > the M-C pushlog by date, I'll have a look. The regression range appears to > be 24 hours only. Maybe. https://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2016-09-08&enddate=2016-09-09+04%3A0%3A00 I guess the culprit(nsAutoCompleteController.cpp) is c35d8f056f67 Marco Bonardo — Bug 1292310 - Enter may wrongly confirm a mouse selected entry in the urlbar. r=adw

Marco, your changes in bug 1292310 seem to have affected the address entry in the Thunderbird addressing widget. We can no longer confirm an address with the enter key that is selected with the cursor keys or by hovering the mouse cursor over it. At least the former is an essential feature, we've had complaints about the latter, so no harm done if the mouse hovered one is no longer selected with enter. Can you please advise.

interesting, could you please point me to the tb code that contains this widget? I mostly would like to know which attributes are set on the input field. (In reply to Jorg K (GMT+2, PTO during summer) from comment #7) > We can no longer confirm an address > with the enter key that is selected with the cursor keys or by hovering the > mouse cursor over it. The latter was a bug afaik, hovering an item with the mouse and pressing enter is not expected to confirm that entry, otherwise it's very easy to get the wrong result by touching a touchpad or having the cursor in the area where the popup appears. The former is not expected clearly, selecting an entry with the keyboard and confirming it should always confirm that entry, and we should figure out why now it behaves wrongly in TB and not in FF.

I suspect your form has completeDefaultIndex but not completeSelectedIndex? and then we don't enter this anymore: if (aIsPopupSelection || (!completeSelection && !defaultCompleted)) correct?

(In reply to Jorg K (GMT+2, PTO during summer) from comment #7) > We can no longer confirm an address > with the enter key that is selected with the cursor keys or by hovering the > mouse cursor over it. At least the former is an essential feature, we've had > complaints about the latter, ... Further reading on this: Bug 1192739. Repeating: No harm done when the last hovered address can't be confirmed with <tab> or <enter> any more, but using the arrow keys should allow to change the selection. The perfect solution would be to get the FF behaviour into TB, that is, if you use the arrow key, the field content is actually changed. Bug 1192739 says: === Firefox does the following: - Hovering changes the "selected" value and does not update the "filled" value. - Arrow keys and tab change the "selected" value *and* the "filled" value is updated. - Enter confirms the "filled" value. === We'd like that!

(In reply to Jorg K (GMT+2, PTO during summer) from comment #10) > (In reply to Jorg K (GMT+2, PTO during summer) from comment #7) > > We can no longer confirm an address > > with the enter key that is selected with the cursor keys or by hovering the > > mouse cursor over it. At least the former is an essential feature, we've had > > complaints about the latter, ... > Further reading on this: Bug 1192739. Repeating: No harm done when the last > hovered address can't be confirmed with <tab> or <enter> any more, but using > the arrow keys should allow to change the selection. The perfect solution > would be to get the FF behaviour into TB, that is, if you use the arrow key, > the field content is actually changed. for this you should only add completeSelectedIndex="true" attribute to the input field. Btw, I can look into the regression and add a test case for this, that looks like we are missing.

Marco, that's for the feedback. There was a mid-air collision. (In reply to Marco Bonardo [::mak] from comment #8) > interesting, could you please point me to the tb code that contains this > widget? I mostly would like to know which attributes are set on the input > field. I'll have to attend to some other issues, I'll get back to you asap, also re. comment #9. > The latter was a bug afaik, hovering an item with the mouse and pressing > enter is not expected to confirm that entry, otherwise it's very easy to get > the wrong result by touching a touchpad or having the cursor in the area > where the popup appears. YES, agreed 100%, see bug 1192739. > ... selecting an entry with the keyboard and > confirming it should always confirm that entry, and we should figure out why > now it behaves wrongly in TB and not in FF. YES, that would be excellent, indeed. See comment #10.

(In reply to Marco Bonardo [::mak] from comment #9) > I suspect your form has completeDefaultIndex but not completeSelectedIndex? Sadly I know close to nothing about autocomplete, Magnus is our resident expert. (I'm just the poor guy who chased bug 1042561 with you.) I looked for completeDefaultIndex and completeSelectedIndex and couldn't find either. Where would they need to go? Here? https://dxr.mozilla.org/comm-central/rev/39761f94a020fa794f720670e873bb96c6df6b45/mail/components/compose/content/messengercompose.xul#955

yes: completedefaultindex="true" forcecomplete="true" minresultsforpopup="2" ignoreblurwhilesearching="true" thanks

This patch restores the previous behaviour, <tab> and <enter> confirm the hovered or cursor selected choice. Using the cursor keys does *not* populate the base field from the popup value.

Wires crossed again, sorry, I'll try what you said in comment #14.

Oops, silly me, we already had: completedefaultindex="true" forcecomplete="true" minresultsforpopup="2" ignoreblurwhilesearching="true" Sorry, something is wrong here. My local build behaves differently to a Daily build (I think I had it refreshed recently. I'll have to look into it).

OK, I got to the bottom of the confusion. While the base field is not fully populated, that is, it still has the >> in it, a hover + <enter> *will still* select the hovered popup value or the one selected via the error keys. If the base field has already been "fixed", no >> in it, then enter will no longer confirm the hovered popup value or the one selected via the arrow keys. That's changed. I guess FF doesn't do the >> business. I'll try with completeSelectedIndex="true" instead of completedefaultindex="true".

OK, I'm trying with completeselectedindex="true" *only*. The effect is that using the cursor keys, the base field is now updated from the popup value. That's how we like it. But shock horror, enter still confirms another value, the first one in the popup list, which is neither the base value nor the hovered one. Even clicking in the popup confirms the first one in the list. So that's absolutely not working. BTW, with this setting, there is none of this >> business. I think Magnus needs to look into it.

This doesn't work at all, see previous comment.

Just repeating (sorry): We'd love to use completeselectedindex="true" to populate the base field from the popup when the arrow keys are used, but enter and click of course need to work. BTW, that >> business, that's an M-C feature, right?

I don't know if we still need that >> feature. We have places in code that have to take care to remove it first if found.

While the >> was visible, I hovered over Laurence and hit enter and she got selected. That's the previous behaviour.

Further to comment #19, as you can see, not the base value, neither the selected value from the popup is used, but the first value in the popup.

Firefox claims that the videos are corrupt, but they play fine in VLC when downloaded.

Sorry about all the noise, but let me summarise the issues seen in Thunderbird: completedefaultindex="true" (current state): 1) If no >> is showing: Cursor-key selected popup value cannot be confirmed with <enter> or <tab>. (original report in this bug, comment #0). 2) If >> is showing: Hovered popup value is still selected with <enter> or <tab>. (video: attachment 8791678 [details]). Accepted TB behaviour but apparently a bug. TB complaints in bug 1192739. completeselectedindex="true" (no >> seem to occur) (desirable future state): 3a) Cursor-key selected popup value is not confirmed with enter or tab (1st in popup is used) (video: attachment 8791806 [details]). 3b) Clicked popup value is not used (1st in popup is used). Marco, please let me know if you want to split off the various issues into separate bugs.

(In reply to Jorg K (GMT+2, never had any PTO during summer) from comment #26) > Marco, please let me know if you want to split off the various issues into > separate bugs. Here I'm just going to take care of the regression, so will fix up case where defaultcompleteindex="true" and there's no completeselectedindex. This is what broke and what thunderbird uses. Other issues or improvements should be moved elsewhere. comments inline: > completedefaultindex="true" (current state): > 1) If no >> is showing: Cursor-key selected popup value cannot be confirmed > with <enter> or <tab>. > (original report in this bug, comment #0). this is what I can fix here. > completeselectedindex="true" (no >> seem to occur) (desirable future state): autocomplete-in-the-middle (the ">>" results) is something that exists only in TB, removing it could allow simplifying some code. I'm all-in for that, but it will require someone to evict that functionality from nsAutoCompleteController and eventual tests. I'm available to review changes, but it will require a separate bug and a decision on your side. > 3b) Clicked popup value is not used (1st in popup is used). This is strange, does it happen always, only for autocomplete-in-the-middle? May need a new bug, since it's quite unexpected.

Filed bug 1303303 for problem 2) and bug 1303304 for problem 3).

Comment on attachment 8791678 [details] Video showing that the previous behaviour hasn't changed in some cases. Attachment not relevant for this bug.

Comment on attachment 8791806 [details] Video showing that completeselectedindex doesn't select the right value. Attachment not relevant for this bug.

Reporter, please note: We have landed bug 1303304 today which changes the configuration of autocomplete used in Thunderbird. Whilst this bug here isn't fixed using the previous configuration, a side effect of the new configuration is that the problem described in comment #0 no longer occurs. Please read bug 1303304 comment #16 for more details. Try it in tomorrow's Daily.

thanks very much Jorg; yes, I've been following the other bugs and saw the checkin. Will test when next Daily available.

looks good to me. Just tested Daily 0919; the issue I originally reported is no longer reproducible. Also, nice to see the fill on cursor movement. Great work, thanks again Jörg.

hope this is the right resolution state.

You misunderstood comment #31. This bug is now about fixing the Mozilla core toolkit autocomplete. We made it work for Thunderbird in bug 1303304 by changing from a configuration that doesn't work and still needs to be fixed, to one that works and has other benefits.

sorry Jörg; I didn't misunderstand it, I forgot it :) apols for the bug spam.

Sorry, I'm really confused now: Attachment: Bug 1302472 - keyboard selected value in autocomplete popup cannot be confirmed with <enter> if completedefaultindex is false but completeselectedindex is true. This bug here is about completedefaultindex="true" only (see summary), which was the previous TB usage. Bug 1303304 was about the experiment to have completeselectedindex="true" before we now have both equal true.

I gave this patch a quick test: Current TB config: completedefaultindex="true" forcecomplete="true" completeselectedindex="true" Unaffected. Previous TB config: completedefaultindex="true" forcecomplete="true" Cursor-key-selected entry can now be confirmed again with <enter>. As noted as TODO in the patch, mouse-over choice can also be confirmed with <enter>, which is subject to bug 1303303. In fact, this patch here seems to aggravate bug 1303303, since without the patch, the mouse-hovered popup item can only be confirmed when autocomplete-in-the-middle is active, now it can be confirmed regardless.

(In reply to Jorg K (GMT+2) from comment #39) > completedefaultindex="true" forcecomplete="true" > Cursor-key-selected entry can now be confirmed again with <enter>. As noted > as TODO in the patch, mouse-over choice can also be confirmed with <enter>, > which is subject to bug 1303303. In fact, this patch here seems to aggravate > bug 1303303, since without the patch, the mouse-hovered popup item can only > be confirmed when autocomplete-in-the-middle is active, now it can be > confirmed regardless. but this is the same that was happening before bug 1292310, right? Cause I'm just undoing the fix in that bug, I figured that the backend can't really distinguish a mouse selection from a keyboard selection if completeselectedindex is false. So we can't fix that without first changing the widget. That is something undergoing a different approach elsewhere.

(In reply to Marco Bonardo [::mak] from comment #40) > but this is the same that was happening before bug 1292310, right? Sure. You reinstated the pre-bug 1292310 behaviour in TB. > Cause I'm just undoing the fix in that bug, ... Completely? You're not bringing the FF problem back you tried to fix in bug 1292310? > I figured that the backend can't really > distinguish a mouse selection from a keyboard selection if > completeselectedindex is false. So we can't fix that without first changing > the widget. That is something undergoing a different approach elsewhere. Well, I'm glad we added completeselectedindex="true" to TB in bug 1303304. That has fixed a lot of problems for us (and since this is the combination FF is using hopefully avoids further problems). Re. comment #38: That patch really as a wrong name, doesn't it?

(In reply to Jorg K (GMT+2) from comment #41) > > Cause I'm just undoing the fix in that bug, ... > Completely? You're not bringing the FF problem back you tried to fix in bug > 1292310? The urlbar takes another code path due to completeselectedindex. It's a partial revert that still fixes the problem on fields having completeselectedindex but reverts to the previous on fields not having it. > Re. comment #38: That patch really as a wrong name, doesn't it? Ah yes, I inverted the two! will fix the patch name. thanks.

Comment on attachment 8792802 [details] Bug 1302472 - keyboard selected value in autocomplete popup cannot be confirmed with <enter> if completedefaultindex is true but completeselectedindex is false. https://reviewboard.mozilla.org/r/79714/#review78884

Pushed by mak77@bonardo.net: https://hg.mozilla.org/integration/autoland/rev/27435af2becb keyboard selected value in autocomplete popup cannot be confirmed with <enter> if completedefaultindex is true but completeselectedindex is false. r=adw

I reproduced this bug using Fx 51.0a1, build ID: 20160909104831, on Windows 10 x64. I can confirm this issue is fixed, I verified using Fx 50.0b5, build ID: 20161006105459 and Fx 52.0a1, build ID: 20161006030208, on Windows 10 x64, mac OS X 10.10.5 and Ubuntu 14.04 LTS. Cheers!