User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:62.0) Gecko/20100101 Firefox/62.0 Steps to reproduce: 1. In address book, New List, go to field to enter email addresses 2. Start typing, an overlay with a match candidate appears. 3. Press enter Actual results: Nothing, the key is ignored. Tab is accepted Expected results: The selection is accepted, cursor moves to next entry field.

Confirmed. I need to look into that.

Alice, perhaps a regression range would be helpful here. Can you find it for us please. Note bug 1500411. Posting on BMO is somewhat broken, you need to repeat until it works. STR: Edit a mailing list. Start typing a new member and wait for the auto-complete to happen. Press enter. Nothing happens, auto-complete should have accepted the item.

I'm utterly puzzled by this bug. Here's what I've done. For address entry, we use autocomplete there: https://searchfox.org/comm-central/rev/f9a9b8d71d4898fef36bc6eea2dfde0e0e56ff35/mail/components/compose/content/messengercompose.xul#1962 I've added debug to <method name="handleEnter"> in M-C's autocomplete.xml and to our functions awRecipientKeyDown, awRecipientKeyPress and awRecipientTextCommand. When I start address entry and navigate the popup with the arrow keys and then hit enter, I get this output: === awRecipientKeyDown === awRecipientKeyPress === handleEnter === awRecipientTextCommand === awRecipientTextCommand All working. However, on the mailing list edit here https://searchfox.org/comm-central/rev/f9a9b8d71d4898fef36bc6eea2dfde0e0e56ff35/mail/components/addrbook/content/abEditListDialog.xul#59 I get this debug, and I can hit enter as many times as I like, it doesn't register at all: === awRecipientKeyDown Well, hitting enter does give another one of those debug lines. So who eats my enter? The XUL for both autocomplete setups is vaguely the same, so I really don't see why one works and the other one doesn't. Marco, do you see anything I don't see?

(In reply to Jorg K (GMT+2) from comment #2) > Alice, perhaps a regression range would be helpful here. Can you find it for > us please. > Note bug 1500411. Posting on BMO is somewhat broken, you need to repeat > until it works. > > STR: > Edit a mailing list. > Start typing a new member and wait for the auto-complete to happen. > Press enter. Nothing happens, auto-complete should have accepted the item. https://hg.mozilla.org/comm-central/pushloghtml?fromchange=5620690a17267e56e1407ae64294624747701ea1&tochange=91311b1d6aa3bddd427858cb7f8c149a81c71a92 https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2c000486eac466da6623e4d7f7f1fd4e318f60e8&tochange=b03d9c87bbf6b9b73883758ce494dd971368b5c9 Suspect: c0b4ca69376c Masayuki Nakano — Bug 1434837 - Make autocomplete and satchel listen to keypress event at the system event group r=mak

Wow, that broke in February 2018 and no one noticed :-( - Thank you so much, Alice! Let's add Masayuki-san here.

I must have don't something wrong, local backout of rev c0b4ca69376c *does* restore the function. Strangely enough TB uses autocomplete in a few places which are all accepting <enter>, only two usages don't work. These two don't have the oninput= that the other call sites have.

Actually, the two panels where it doesn't work are modal, all the others aren't.

Fixed for TB 60 ESR via backout: https://hg.mozilla.org/releases/mozilla-esr60/rev/92515b2d8a74ec96119209101319a8b21e06c44d on THUNDERBIRD_60_VERBRANCH Backed out changeset c0b4ca69376c (bug 1434837 for causing bug 1499410) to build Thunderbird 60.3. a=jorgk

I fear I'm not of help here, this seems to require specific debugging, you could maybe try to build a simple testcase of an autocomplete in a modal dialog to see if it can be used to reproduce the problem.

Well, I launched the panel non-modal and the autocomplete still doesn't work. As I said, I'm utterly puzzled that very similar XUL configuration works in all other cases. Looks like Masayuki-san needs to shed some light on the event processing here.

Well, backing out bug 1434837 fixes this bug means that there are some Thunderbird specific "keypress" event listeners in the default group and they are added after autocomplete/satchel adds "keypress" event listener. So, previously, autocomplete/satchel consumed "keypress" events before them. However, now, the other listeners handle "keypress" event first, then, autocomplete/satchel's listener handles it later. You need to look for the "keypress" event listener and make it into the system group (use addSystemEventListener or addEventListener with {mozSystemGroup: true}).

Hmm, so if I understand you correctly, there is a conflict between a listener we register and the autocomplete listener and our listener runs first and messes up the event. I thought I had found the issue in abMailListDialog.js we indeed we add listeners to the now failing panel. So I changed them to document.addEventListener("keypress", awDocumentKeyPress, {capture: true, mozSystemGroup: true}); but that didn't help. Next I took out various listeners I could see in DXR, which I admit doesn't make much sense and also has no effect. So how can I find the offending listener? We have heaps of hits on "keypress" in C-C: https://dxr.mozilla.org/comm-central/search?q=keypress+path%3Acomm%2F+-path%3Acomm%2Fcalendar%2F&redirect=false https://dxr.mozilla.org/comm-central/search?q=regexp%3Aaddeventlistener.*keypress+path%3Acomm%2F+-path%3Acomm%2Fcalendar%2F&redirect=false

(In reply to Jorg K (GMT+2) from comment #15) > Created attachment 9019031 [details] [diff] [review] > 1499410-fix-mailinglist-enter.patch - WIP, experiment, not working > > Hmm, so if I understand you correctly, there is a conflict between a > listener we register and the autocomplete listener and our listener runs > first and messes up the event. Yes, I think so. Or, another possible case is, another keypress event listener is added by toolkit or something but the combination is not used in m-c. > I thought I had found the issue in abMailListDialog.js we indeed we add > listeners to the now failing panel. So I changed them to > document.addEventListener("keypress", awDocumentKeyPress, {capture: true, > mozSystemGroup: true}); > but that didn't help. Hmm, it's anyway odd. It listens "Enter" keypress *before* autocomplete module since it adds the event listener to capturing phase of the document node, but autocomplete attaches the editable element. I must not realize something... > Next I took out various listeners I could see in DXR, which I admit doesn't > make much sense and also has no effect. So how can I find the offending > listener? I also want to know... I always read implementation a lot when I meet unexpected orange at working on such changes... If the consumer is in C++, you can get the stack with MOZ_ASSERT(); in dom::Event::PreventDefault() and/or dom::Event::StopProparation(). E.g., MOZ_ASSERT(!mEvent || !mEvent->AsKeyboardEvent() || mEvent->AsKeyboardEvent()->mKeyNameIndex != KEY_NAME_INDEX_Enter); but I guess it's in JS... > We have heaps of hits on "keypress" in C-C: > https://dxr.mozilla.org/comm-central/search?q=keypress+path%3Acomm%2F+- > path%3Acomm%2Fcalendar%2F&redirect=false > https://dxr.mozilla.org/comm-central/search?q=regexp%3Aaddeventlistener. > *keypress+path%3Acomm%2F+-path%3Acomm%2Fcalendar%2F&redirect=false Or, I wonder, "accept" handler of XUL dialog might not work with autocomplete.

I do not understand the code, but If I change abMailListDialog.xul as follows, then the mailing list autocomplete works as expected. <textbox id="addressCol1#1" class="plain textbox-addressingWidget uri-element" type="autocomplete" flex="1" autocompletesearch="addrbook ldap" autocompletesearchparam="{}" timeout="300" maxrows="4" completedefaultindex="true" forcecomplete="true" completeselectedindex="true" minresultsforpopup="3" - ontextentered="awRecipientTextCommand(param, this)" + ontextentered="awRecipientTextCommand(param, this);event.preventDefault();" - onkeypress="awHandleKeyPress(this, event);" onkeydown="awRecipientKeyDown(event, this);" onclick="awNotAnEmptyArea(event);"> <image onclick="this.parentNode.select();" class="person-icon"/>

(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #16) > Hmm, it's anyway odd. It listens "Enter" keypress *before* autocomplete > module since it adds the event listener to capturing phase of the document > node, but autocomplete attaches the editable element. I must not realize > something... As you see in the patch, even removing the listener completely doesn't fix the issue. (In reply to Alice0775 White from comment #17) > - ontextentered="awRecipientTextCommand(param, this)" > + ontextentered="awRecipientTextCommand(param, this);event.preventDefault();" > - onkeypress="awHandleKeyPress(this, event);" Thanks Alice, I'll play with this. Note that his pattern is used elsewhere and it works: https://searchfox.org/comm-central/rev/f9a9b8d71d4898fef36bc6eea2dfde0e0e56ff35/mail/components/compose/content/messengercompose.xul#1962 So there must be other circumstances that lead to this malfunction.

Alice's suggestion - onkeypress="awHandleKeyPress(this, event);" gets us closer to the issue. With just that change, hitting enter accepts the autocomplete choice, and closes the panel. To avoid panel closing, that is, passing the event to the OK button, the function does this: https://searchfox.org/comm-central/rev/ed8a923565e8de31d976ad58f0dc95850c0da275/mailnews/addrbook/content/abMailListDialog.js#28 function awHandleKeyPress(element, event) { // allow dialog to close on enter if focused textbox has no value if (element.value != "" && event.keyCode == KeyEvent.DOM_VK_RETURN) { event.stopPropagation(); event.preventDefault(); } } So now this function is eating the DOM_VK_RETURN and it doesn't get to the auto-complete any more. So it appears to be XUL's own onkeypress handler that now gets the event first before the autocomplete gets it. Masayuki-san, what can we do about that? That doesn't appear right. Shouldn't autocomplete as the top most "action" get the event first? Or I need to change awHandleKeyPress() to pass the event onto the autocomplete widget somehow. I think I also understand why Alice's further suggestion works: - ontextentered="awRecipientTextCommand(param, this)" + ontextentered="awRecipientTextCommand(param, this);event.preventDefault();" So once we remove the call to awHandleKeyPress(), the autocomplete gets the event, and eventually ontextentered gets run. With preventDefault() added, it will prevent the closing of the panel. So with Alice's solution we can no longer close the panel with enter now when nothing got added.

This works. It's basically Alice's idea with an improvement. I killed the doubtful and badly named awHandleKeyPress() that cancelled DOM_VK_RETURN but assumed that auto-complete had already processed it (which is no longer true). Instead, if a value was entered, I follow Alice's idea to prevent the default so the panel isn't closed. Is no value was entered, the event will go to the OK button and close the panel. Maybe we can take this to the ESR as well and backout the backout of the Mozilla patch ;-)

Comment on attachment 9019496 [details] [diff] [review] 1499410-fix-ml-enter.patch (v1) Review of attachment 9019496 [details] [diff] [review]: ----------------------------------------------------------------- Thanks, seems to work for me. ::: mail/components/addrbook/content/abMailListDialog.xul @@ +78,1 @@ > onkeypress="awHandleKeyPress(this, event);" Forgot to remove this one?

(In reply to :aceman from comment #21) > > onkeypress="awHandleKeyPress(this, event);" > Forgot to remove this one? Oops. It still works since I removed the function :-)

Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/d3daa2594870 fix mailing list enter handling (idea by Alice White). r=aceman

Beta (TB 64): https://hg.mozilla.org/releases/comm-beta/rev/dfaefe83ca91

TB 60.3: https://hg.mozilla.org/releases/comm-esr60/rev/ff097eea51d576adda0f75c2b28d29ee40100eaa Undid the backout from comment #11 here: https://hg.mozilla.org/releases/mozilla-esr60/rev/a3f82738156799bf9bf3948f54f40a1b0a1fb4f2

(In reply to Jorg K (GMT+2) from comment #20) > This works. It's basically Alice's idea with an improvement. I killed the > doubtful and badly named awHandleKeyPress() that cancelled DOM_VK_RETURN but > assumed that auto-complete had already processed it (which is no longer > true). Makes sense. (In reply to Jorg K (GMT+2) from comment #19) > So it appears to be XUL's own onkeypress handler that now gets the event > first before the autocomplete gets it. Masayuki-san, what can we do about > that? That doesn't appear right. Shouldn't autocomplete as the top most > "action" get the event first? Or I need to change awHandleKeyPress() to pass > the event onto the autocomplete widget somehow. Really good point. The reason why autocomplete should handle keyboard events in the system group is, autocomplete is one of default action. Therefore, it should handle keyboard events after any event listeners in web content. And another reason is, keypress events won't be fired for non-printable keys unless the document is in chrome document. So, in other words, in chrome XUL document, we could make autocomplete handle keyboard events in the default group for backward compatibility. However, on the other hand, we don't have any similar bugs for now. If we'd get similar reports, it could be better to change the group (I think that we're just lucky in this bug since you succeeded to fix this bug without too big/risky changes).

(And really sorry for the delay. Currently, my queue is full. I'll reply each your ni request later.)