Open Bug 286933 Opened 18 years ago Updated 2 months ago
Key events in the autocomplete popup should be hidden from page scripts
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050319 Firefox/1.0+ On search.msn.com, when you enter a search term that brings up the autocomplete box, and you use arrows to select the entry you want to autocomplete and then press enter, MSN search captures what you entered into the search box, not the entry that was autocompleted. Steps to Reproduce: 1. http://search.msn.com/ 2. Start typing a partial search term that will bring up the autocomplete box 3. Arrow key down to the thing you want to select 4. Hit enter Expected Results: Assume your term in step 2 was 'moz', and from step 3 'mozilla', it should perform a search on 'mozilla'. Actual Results: It performed a search on 'moz'. We should hide the autocomplete enter key press from page scripts. Jesse thinks this might be a regression from bug 270697, so I'm marking it as such.
I still see this bug at http://search.msn.com/ (as described in comment 0) using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051005 Firefox/1.6a1.
To reproduce with the testcase, save the testcase and use the local copy. The testcase's behavior gets even more strange when it is loaded from https.
*** Bug 297329 has been marked as a duplicate of this bug. ***
I believe that this bug is related to Bug #311795. Both bugs were once fixed on the 1.0.x branch and have resurfaced sometime before 1.5b1 was released and now exist everywhere.
I believe that this bug is related to Bug #311795. Both bugs were once fixed on the 1.0.x branch and have resurfaced sometime before 1.5b1 was released and now exist everywhere. They seem to point to a more general problem of keypresses being sent to page scripts instead of the autocomplete dropdown.
With the latest branch builds on Windows XP I still see this. 1. Get test, testing, and testing123 in the autocomplete history. 2. Type "te" into the form and arrow down to "testing123". 3. Hit <enter>. Note that "te" was searched for instead of "testing123".
I still cannot reproduce with those steps on today's build on windowsXP. I don't think we can block on a bug like this that's not easily reproduced.
Flags: blocking1.8rc1? → blocking1.8rc1-
Asa, I can confirm this, although I'm not knowledgeable enough to tell if it's the way JS is being (ab)used by the various sites or the way we're interpreting things. However, using the steps in comment 9 (at either staples.com or search.msn.com as two examples) I do notice that although the browser renders the full "testing123" into the search box and then loads the results page, the actual submitted search term is "te"!
Renominating for 1.8rc1 - I've been able to recreate this with new and existing profiles, and the behaviour is (unintentionally) sneaky enough to actually make a user thing that they've searched for "football" when, in fact, they've searched for "foo".
Flags: blocking1.8rc1- → blocking1.8rc1?
I can repoduce this also. In an autocomplete list at search.msn.com "Enter" submits what's typed in the search field, "enter should behave like clicking on the autocomplete item; it fills in the search field. Doing steps from comment 9 at google.com does what is expected. A second "Enter" is then required to submit the search. (or click the search button)
Tracy, can you get us a regression window so we can get blame for it? Thanks.
Jesse said he thought this might be caused by bug 270697. I checked and this regressed between 2005-02-17 07:00 and 2005-02-18 07:00, during which the patch for bug 270697 was checked in. http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=PhoenixTinderbox&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-02-17+07&maxdate=2005-02-18+07&cvsroot=%2Fcvsroot
Dan, this regression appeared when you fixed bug 270697, "Autocomplete data leak".
QA Contact: davidpjames → location.bar
bryner and johnny, can you guys look at this? Dveditz, can you put your analysis here so bryner and jst can see what you've determined so far.
The modified testcase is identical to the first except that the submit is called in a timeout and the event is passed on. Comments mark the changed lines.
I wasn't saying that we should back bug 270697 out, but that pages shouldn't get key events that are directed at autocomplete menus. When you're navigating within autocomplete, it could be argued that you're interacting with browser UI, so pages shouldn't get events just like they shouldn't get events when you navigate browser menus.
(In reply to comment #20) > When you're navigating within autocomplete, it could be argued that you're > interacting with browser UI, so pages shouldn't get events just like they > shouldn't get events when you navigate browser menus. It's a reasonable argument, but the last days of 1.5 isn't the time to re-write the way all of our XBL-based widgets work.
The calendar pages on yahoogroups.com now succumb to this behavior as well. I haven't checked out the other pages on the same website. I'm not sure if I should file a new evangelism bug for each new site I come across that displays this behavior, so sorry if I'm spamming. I rather think it's a UI issue, but maybe I shouldn't report every site that offends, either. Please advise. Thanks.
Component: Location Bar and Autocomplete → Autocomplete
Product: Firefox → Toolkit
Hardware: PC → All
I don't think page events should be able to eat the event pointed at the autocomplete widget. We can either match IE's behaviour, or we can try to TE the world. In this case, IE does the right thing, IMO, so we need to find a way to do that right.
Flags: blocking-aviary2? → blocking-aviary2+
*** Bug 322658 has been marked as a duplicate of this bug. ***
I agree with Mike Connor. FF can't ask every site to submit every form on a timeout just because autocomplete might be enabled. Further, users probably don't want selecting an autocomplete item (by pressing enter) to submit the form right away. What if the autocomplete was for the first field out of several needing to be filled out?
*** Bug 342342 has been marked as a duplicate of this bug. ***
Throws an excpetion to the JS console, by the way: Permission denied to set property XULElement.selectedIndex' when calling method: [nsIAutoCompletePopup::selectedIndex]" nsresult: "0x8057001e (NS_ERROR_XPC_JS_THREW_STRING)" location: "JS frame... For what it's worth, I agree Autocomplete events belong to chrome not to the user. If the page author wants to capture Enter it is probably to submit the form. A useful script for the current FF would need to know if Enter came from Autocomplete or not. The meaning of Enter is not the same in Autocomplete. So using Timeout just corrects one part of the problem, but replicates the other problem, a little later. The user did not use Enter to mean "Submit" or "Done". It just meant "select this item." If the user used a mouse click to select the Automcomplete item, the same meaning, it does not trigger the event.
Target Milestone: mozilla1.8.1beta1 → mozilla1.8.1beta2
Moving to nom list for reconsideration, likely to be minused if we can't get traction really soon.
Flags: blocking-firefox2+ → blocking-firefox2?
Why is the timeout necessary? Switching the event handler from capturing to bubbling seems to have fixed the testcase fairly easily. (though we should still eat the enter keypress event)
We're not going to block on this at this point, but will consider taking a fix if a safe patch appears in the next day or so.
Flags: blocking-firefox2? → blocking-firefox2-
This should be safe to do since the keypress handler bails out if there's no input focused (!mFocusedInput).
Assignee: nobody → mano
Status: NEW → ASSIGNED
Attachment #239184 - Flags: first-review?(mconnor)
Severity: normal → major
Priority: -- → P2
Target Milestone: mozilla1.8.1beta2 → mozilla1.9alpha1
Comment on attachment 239184 [details] [diff] [review] patch r=me simple and sane, thanks for fixing this!
Attachment #239184 - Flags: first-review?(mconnor) → first-review+
mozilla/toolkit/components/satchel/src/nsFormFillController.cpp 1.75 mozilla/toolkit/components/satchel/src/nsFormFillController.h 1.14
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Is this safe enough for 18.104.22.168?
Comment on attachment 239184 [details] [diff] [review] patch Approved for 1.8 branch, a=jay for drivers.
Attachment #239184 - Flags: approval22.214.171.124? → approval126.96.36.199+
1.8 branch: mozilla/toolkit/components/satchel/src/nsFormFillController.cpp 188.8.131.52 mozilla/toolkit/components/satchel/src/nsFormFillController.h 184.108.40.206
Verified Fixed for 220.127.116.11 - after tests with Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:18.104.22.168pre) Gecko/20070129 BonEcho/22.214.171.124pre ID:2007012903 on XP x64 and on Linux Fedora FC 6
Backed-out from both trunk and branch (with a=dveditz), nice try I say. :-/
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Assignee: mano → nobody
Status: REOPENED → NEW
Target Milestone: mozilla1.9alpha1 → ---
Added similar bug 364733, which is about keydown (patches here seem to only affect the keypress event).
This is a 6-year old bug. Please fix it. It completely ruins user experience that the Enter key of Autocomplete is propagated to the form _before_ the autocomplete value is set in the field. Either block this Enter completely from reaching the user's script or at least fill the field with the autocomplete value before propagating the event.
(In reply to White Tiger from comment #43) > This is a 6-year old bug. Please fix it. It completely ruins user experience > that the Enter key of Autocomplete is propagated to the form _before_ the > autocomplete value is set in the field. Either block this Enter completely > from reaching the user's script or at least fill the field with the > autocomplete value before propagating the event. Not really sure if filling the field before propagating to the user's script would be a solution. In case the user is logging in a website(usually email and password) he would get a wrong password message just for using the autocomplete. But it should be patched, anyway.
This is still a bug in Firefox 9.0.1. I have an onkeypress event on the form which is generated by ASP.NET. If users press enter to select an item from their autocomplete, it submits the incomplete form. This is terrible for user registration forms. Any keypress events within an autocomplete dropdown should NOT propagate to the user scripts in any way.
This is still a bug in Firefox 14.0.1 The keys used to navigate and select an item from the drop down list of autocomplete choices should not be sent as DOM keypress or possibly keydown events. Sending as keyup is OK. The enter key is the most problematic since it is commonly used to submit a form. I tested to see which events could detect the enter key when pressed to select an autocomplete choice in a few browsers: IE 9: keydown, keyup Chrome 21: keyup Firefox 14: keydown, keypress, keyup This leaves now way in Firefox to distinguish the enter key used to select an autocomplete choice from an enter key pressed while in the text input. My code is currently checking for enter in keypress so it works with autocomplete in Chrome and IE but not Firefox. I think the behavior in chrome makes the most sense. The workaround requires the user to use right arrow rather than enter to select an autocomplete choice.
Press ENTER in the dorpdown list(list item/autocomprete) fires keypress event.
Yes, and for some reason I see more and more pages affected lately. I'm going to try to make my patch from 2006 work without reintroducing bug 372015.
By the way, a similar issue applies to <select>s. See eBay's advanced search page for some (very annoying!) examples.
Possible duplicates: bug 692581, bug 175597, bug 286933, bug 364733, bug 819616 Reportedly observed on these pages: https://secure.feedingamerica.org/site/Donation2?df_id=1560&1560.donation=form1 https://www.ups.com/one-to-one/login https://www.tumblr.com/login https://course.bighistoryproject.com/en/Sign-In https://idmsa.apple.com/IDMSWebAuth/authenticate.html https://artemis.siol.net/servicePages/validate-login.sp http://search.msn.com/ http://www.dotnetnuke.com/Home/tabid/510/ctl/Login/Default.aspx http://paypal.com
Informations and another discussion about this bug can be found here too: http://stackoverflow.com/q/2303955
From poking at this for half an hour: this is all fun and games because we have approximately these requirements (caveat emptor per the previous indication of how long I looked at this): 1) if the popup isn't open, listen for the key event in the default action phase, and only then open the popup if it's not been preventDefaulted 2) if the popup is open, listen for the key event as early as possible (probably on window, as a capturing event listener, potentially system group?) and kill the event then and there if and only if we act on it 3) do this for all of keydown, keypress and keyup, but don't act twice (e.g. single key down / key press / key up should move only 1 item down in the autocomplete popup, but we should stop all 3 events from reaching the webpage) 4) do this for both the handling inside the autocomplete popup binding (without leaking the world if the binding is destroyed / disconnected) and nsFormFillController I do think we should fix this sooner rather than later, so I'm setting backlog+. Dolske, can you adjudicate whether this should be prioritized more than just that? :-)
(In reply to :Gijs Kruitbosch from comment #56) > 1) if the popup isn't open, listen for the key event in the default action > phase, and only then open the popup if it's not been preventDefaulted (from: https://bugzilla.mozilla.org/show_bug.cgi?id=633058#c8 )
(In reply to :Gijs Kruitbosch from comment #56) > I do think we should fix this sooner rather than later, so I'm setting > backlog+. Dolske, can you adjudicate whether this should be prioritized more > than just that? :-) We should fix this, but it doesn't have any particular urgency (he says, commenting on a 9+ year old bug). Might be worth fleshing out comment 56 a bit to make it more actionable by someone who wants to pick it up?
In bug 1121040 I hid the RETURN keypress from pages when an entry is selected in the autocomplete popup. This bug should handle keypress events for other keys and also deal with hiding keydown/keyup from the page at the same time as keypress. See comment 56.
Summary: Enter that selects autocomplete entry should be hidden from page scripts → Key events in the autocomplete popup should be hidden from page scripts
Reported: 13 years ago
Mass bug change to replace various 'parity' whiteboard flags with the new canonical keywords. (See bug 1443764 comment 13.)
Severity: major → --
You need to log in before you can comment on or make changes to this bug.