Ali Ebrahim Reporter
	Description • 20 years ago

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.

	Jesse Ruderman
	Comment 2 • 19 years ago

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.

	Jesse Ruderman
	Comment 4 • 19 years ago

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.

	Adam Guthrie
	Comment 5 • 19 years ago

*** Bug 297329 has been marked as a duplicate of this bug. ***

	tyl2
	Comment 6 • 19 years ago

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.

	tyl2
	Comment 7 • 19 years ago

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.

	Adam Guthrie
	Comment 9 • 19 years ago

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".

	Asa Dotzler [:asa]
	Comment 10 • 19 years ago

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. 

	Mike Beltzner [:beltzner, not reading bugmail]
	Comment 11 • 19 years ago

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"!

	Mike Beltzner [:beltzner, not reading bugmail]
	Comment 12 • 19 years ago

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".

	Tracy Walker [:tracy]
	Comment 13 • 19 years ago

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)

	Asa Dotzler [:asa]
	Comment 14 • 19 years ago

Tracy, can you get us a regression window so we can get blame for it? Thanks.

	Adam Guthrie
	Comment 15 • 19 years ago

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

	Jesse Ruderman
	Comment 16 • 19 years ago

Dan, this regression appeared when you fixed bug 270697, "Autocomplete data leak".

	Asa Dotzler [:asa]
	Comment 17 • 19 years ago

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. 

	Daniel Veditz [:dveditz]
	Comment 18 • 19 years ago

The problem is that the form's key listener code snags the return and submits
the form, and the autocomplete control never gets the enter keypress. If you
turn off Javascript then the autocomplete works correctly. If you change the
event listener to not eat the event then the autocomplete HandleEnter() does get
called and does the right thing populating the field, but the form data has
already been submitted without the new data.

I don't know that this behavior is actually wrong. Clearly it's not what the
page author wants, but I don't think we want to make autocomplete be a magic
widget that gets events regardless of whether page content is trying to claim
them. Are any of our other widgets like that? The problem is the page was
written with the buggy pre-270697 behavior in mind, but I argue strongly that we
shouldn't back that out. It was a privacy security issue, and also didn't behave
like the IE autocomplete.

IE's autocomplete dropdown seems to eat the event -- the page's keypress handler
submits the page only when Enter is pressed and autocomplete popup is closed.

The page can be re-written to work as intended: change the keypress handler to
not eat the event and call the submit in a timeout. Or really, if they simply
dropped the the handler it would behave like IE: if the autocomplete popup is
open Enter fills the field but doesn't submit, and if the popup is closed Enter
submits.

I'd like to call this a Tech Evangelism bug.

	Daniel Veditz [:dveditz]
	Comment 19 • 19 years ago

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.

	Jesse Ruderman
	Comment 20 • 19 years ago

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.

	Daniel Veditz [:dveditz]
	Comment 21 • 19 years ago

(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.

	tyl2
	Comment 22 • 19 years ago

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.

	Mike Connor [:mconnor]
	Comment 23 • 19 years ago

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.

	Adam Guthrie
	Comment 24 • 19 years ago

*** Bug 322658 has been marked as a duplicate of this bug. ***

	Peter Flynn (Oracle)
	Comment 25 • 19 years ago

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?

	kitchin
	Comment 26 • 18 years ago

*** Bug 342342 has been marked as a duplicate of this bug. ***

	kitchin
	Comment 27 • 18 years ago

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.

	Mike Connor [:mconnor]
	Comment 28 • 18 years ago

Moving to nom list for reconsideration, likely to be minused if we can't get traction really soon.

	Michael Wu [:mwu]
	Comment 29 • 18 years ago

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)

	Mike Connor [:mconnor]
	Comment 30 • 18 years ago

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.

	Asaf Romano (gone)
	Comment 31 • 18 years ago

This should be safe to do since the keypress handler bails out if there's no input focused (!mFocusedInput).

	Mike Connor [:mconnor]
	Comment 32 • 18 years ago

Comment on attachment 239184 [details] [diff] [review]
patch

r=me

simple and sane, thanks for fixing this!

	Asaf Romano (gone)
	Comment 33 • 18 years ago

mozilla/toolkit/components/satchel/src/nsFormFillController.cpp 1.75
mozilla/toolkit/components/satchel/src/nsFormFillController.h 1.14

	Ryan VanderMeulen [:RyanVM]
	Comment 34 • 18 years ago

Is this safe enough for 2.0.0.2?

	Jay Patel [:jay]
	Comment 35 • 18 years ago

Comment on attachment 239184 [details] [diff] [review]
patch

Approved for 1.8 branch, a=jay for drivers.

	Asaf Romano (gone)
	Comment 36 • 18 years ago

1.8 branch:
mozilla/toolkit/components/satchel/src/nsFormFillController.cpp 1.51.4.18
mozilla/toolkit/components/satchel/src/nsFormFillController.h 1.11.4.4

	Carsten Book [:Tomcat]
	Comment 37 • 18 years ago

Verified Fixed for 1.8.1.2 - after tests with Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.2pre) Gecko/20070129 BonEcho/2.0.0.2pre ID:2007012903 on XP x64 and on Linux Fedora FC 6

	Asaf Romano (gone)
	Comment 38 • 18 years ago

Backed-out from both trunk and branch (with a=dveditz), nice try I say. :-/

	Nickolay_Ponomarev
	Comment 40 • 15 years ago

Added similar bug 364733, which is about keydown (patches here seem to only affect the keypress event).

	White Tiger
	Comment 43 • 13 years ago

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.

	Leonardo Telles
	Comment 44 • 13 years ago

(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.

	Wilson Pascoal
	Comment 45 • 13 years ago

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.

	John Snyders
	Comment 46 • 12 years ago

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.

	Alice0775 White
	Comment 47 • 11 years ago

Press ENTER in the dorpdown list(list item/autocomprete) fires keypress event.

	Asaf Romano (gone)
	Comment 49 • 11 years ago

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.

	Asaf Romano (gone)
	Comment 50 • 11 years ago

By the way, a similar issue applies to <select>s. See eBay's advanced search page for some (very annoying!) examples.

	Shad Sterling
	Comment 51 • 11 years ago

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

	m.kurz
	Comment 53 • 11 years ago

Informations and another discussion about this bug can be found here too: http://stackoverflow.com/q/2303955

	:Gijs (he/him)
	Comment 56 • 10 years ago
suggestedfix

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? :-)

	:Gijs (he/him)
	Comment 57 • 10 years ago

(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 )

	Justin Dolske [:Dolske]
	Comment 59 • 10 years ago

(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?

	Matthew N. [:MattN]
	Comment 60 • 10 years ago

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.

	Clemens Eisserer
	Comment 61 • 7 years ago

Reported: 13 years ago

	Jonathan Watt [:jwatt]
	Comment 62 • 6 years ago

Mass bug change to replace various 'parity' whiteboard flags with the new canonical keywords. (See bug 1443764 comment 13.)