Right arrow key after selecting autocomplete result no longer uses selected item

RESOLVED FIXED

Status

()

RESOLVED FIXED
14 years ago
14 years ago

People

(Reporter: asa, Assigned: dveditz)

Tracking

({fixed-aviary1.0.5, regression})

unspecified
x86
Windows XP
fixed-aviary1.0.5, regression
Points:
---
Bug Flags:
blocking-aviary1.0.3 -
blocking-aviary1.0.4 -
blocking-aviary1.0.5 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

14 years ago
In html forms we used to be able to select an auto-complete item and hit the
right arrow and it would fill that item placing the caret at the end of that
word. Now hitting the right arrow dismisses autocomplete. IE's right arrow on
autocomplete does nothing. We used to be better and now we're either similar or
worse. It would be good to get this back to being better. Tested with 1.0.1 on
windows XP
For the record, this worked in 1.0.
This is fallout from bug 270697, I assume.
dveditz, do you want to take this?  Ben can certainly fix it, but you were last
to touch and may be best assignee.  Bryner would help too, I bet.

/be

Updated

14 years ago
Keywords: regression
Summary: autocomplete regressed → Right arrow key after selecting autocomplete result no longer uses selected item
(Assignee)

Comment 4

14 years ago
In the spirit of sheer nitpickiness the right-arrow is doing what it has always
done: close the popup and nothing more. The text was already filled in because
selecting an item did it.

This patch restores right-arrow functionality. left-arrow functionality is one
character different: in the past the caret would be one character before the
end, this patch puts it at the end like right-arrow instead. If left-arrow had
done something useful like put the caret at the beginning it might have been
worth trying to preserve, but for one character I think this is OK.
(Assignee)

Updated

14 years ago
Assignee: bugs → dveditz
Status: NEW → ASSIGNED
Attachment #175688 - Flags: superreview?(bryner)
Attachment #175688 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 175688 [details] [diff] [review]
restore old right-arrow behavior

Neil isn't a toolkit peer, and toolkit doesn't require SR.

One thought, is handleEnter really the right name still, now that we're not
just calling it on when pressing Enter?
Attachment #175688 - Flags: superreview?(bryner)
Attachment #175688 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #175688 - Flags: review+

Comment 6

14 years ago
*** Bug 284985 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

14 years ago
Flags: blocking-aviary1.1?
Flags: blocking-aviary1.0.3?
(Assignee)

Updated

14 years ago
Attachment #175688 - Flags: approval-aviary1.1a?
Attachment #175688 - Flags: approval-aviary1.0.3?

Comment 7

14 years ago
Comment on attachment 175688 [details] [diff] [review]
restore old right-arrow behavior

a=chase approved for landing on the trunk
Attachment #175688 - Flags: approval-aviary1.1a? → approval-aviary1.1a+

Comment 8

14 years ago
Comment on attachment 175688 [details] [diff] [review]
restore old right-arrow behavior

approval-aviary1.0.3-, nominating for approval-aviary1.0.4
Attachment #175688 - Flags: approval-aviary1.0.4?
Attachment #175688 - Flags: approval-aviary1.0.3?
Attachment #175688 - Flags: approval-aviary1.0.3-
(Assignee)

Comment 9

14 years ago
checked in to trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Flags: blocking-aviary1.0.4?
Flags: blocking-aviary1.0.3?
Flags: blocking-aviary1.0.3-
Resolution: --- → FIXED
(Reporter)

Updated

14 years ago
Flags: blocking-aviary1.1?
(In reply to comment #9)
> checked in to trunk
could this have caused bug 290172 ?

Comment 11

14 years ago
HandleEnter is no good because in the location bar or search bar, Enter causes
the browser to immediately begin loading a new page.  This is bug 290172. 
Attachment 180740 [details] [diff] to that bug is a reimplementation of the fix for this bug. 
The only difference compared to the committed patch (attachment 175688 [details] [diff] [review]) is that
the left arrow moves the insertion point to one character before the end.  See
comment 4.  I experimented with having the left arrow key move the insertion
point to the beginning of the string, but I didn't like that in cases where
completeselectedindex is set.

This patch approximates the behavior of Firefox 1.0 as nearly as I am able to
discern.  The only difference seems to be that completeselectedindex seems to be
set for text fields in 1.0 and is not set for text fields on the trunk.  (It is
set for the location bar and search bar, though.)

Comment 12

14 years ago
This is a fix for the 1.0.x branch using the same strategy that was used in bug
290172.  It restores the desired side arrow key behavior without making the
location bar unpleasant.  Since the bug is a regression from 1.0 in the 1.0.x
series, mconnor asked me to port that patch from the trunk.
Attachment #180857 - Flags: review?(mconnor)
Attachment #180857 - Flags: approval-aviary1.0.4?
Comment on attachment 180857 [details] [diff] [review]
Patch for AVIARY_1_0_1 branch

not sure if/when we'll have a 1.0.4, but this meets current criteria
(regression from a security fix) and is on trunk already.
Attachment #180857 - Flags: review?(mconnor) → review+
(Assignee)

Comment 14

14 years ago
Comment on attachment 175688 [details] [diff] [review]
restore old right-arrow behavior

Don't want this one
Attachment #175688 - Attachment is obsolete: true
Attachment #175688 - Flags: approval-aviary1.0.4? → approval-aviary1.0.4-
(Assignee)

Updated

14 years ago
Flags: blocking-aviary1.0.4? → blocking-aviary1.0.4+
(Assignee)

Updated

14 years ago
Flags: blocking-aviary1.0.4?
1.0.4 is not taking non-critical security fixes.

/be
Flags: blocking-aviary1.0.4? → blocking-aviary1.0.4-

Updated

14 years ago
Whiteboard: have patch for regression

Comment 16

14 years ago
dveditz:  Do we need an sr= before approving this for 1.0.5?

Updated

14 years ago
Attachment #180857 - Flags: approval-aviary1.0.5? → approval-aviary1.0.5+

Comment 17

14 years ago
Let's get this on the Aviary branch... a=jay
Whiteboard: have patch for regression → need branch landing
landed attachment 180857 [details] [diff] [review] on the aviary1.0.1 branch
Keywords: fixed-aviary1.0.5
Whiteboard: need branch landing
You need to log in before you can comment on or make changes to this bug.