Closed Bug 406179 Opened 12 years ago Closed 12 years ago
delete (or shift/delete) of a row in the rich url bar results does not select the next item after delete
delete (or shift/delete on the mac) of a row in the rich url bar results does not select the next item after delete.
(Regressions that were caused by a fix for a particular bug should be blocking that particular bug)
see also bug 406238
requesting blocking due to it being a regression
Summary: delete (or shift/delete on the mac) of a row in the rich url bar results does not select the next item after delete → delete (or shift/delete) of a row in the rich url bar results does not select the next item after delete
Assignee: nobody → sspitzer
Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P3
Target Milestone: --- → Firefox 3 M11
Component: Places → Location Bar and Autocomplete
QA Contact: places → location.bar
Seth: What's the following bit for: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/content/widgets/autocomplete.xml&rev=1.103&mark=926-937#914 ? That's where the already correctly selected next item loses the selection because the richlistitem of the deleted entry is still hanging around (until _appendCurrentResult overrides it just further down - though a tad too late). Removing it fixes this bug. In case that would cause a different regression, please add a comment to the code itself to make things clearer.
simon, thanks for the detective work. that change (see https://bugzilla.mozilla.org/attachment.cgi?oldid=290322&action=interdiff&newid=290496&headers=1) was made to address the problem of selection persisting between searches. (see https://bugzilla.mozilla.org/show_bug.cgi?id=399664#c99) but, the fix for bug #400671 may have addressed that problem, as we now call this.selectedIndex = -1 when on popuphiding. if so, we might be able to remove the code you've highlighed and fix this bug without regressing anything. but I also need to make sure the problem of additional autocomplete results coming in (and calling invalidate()) while the results are open does not clear the selected item.
Seth: Are you still doing reviews? I've been using this patch for since my comment #7 and haven't noted any regression. Furthermore I've compared the code paths of the two autocomplete popups and I don't see anything similar in the classic implementation. (In reply to comment #8) > but I also need to make sure the problem of additional autocomplete results > coming in (and calling invalidate()) while the results are open does not clear > the selected item. That's not what this code would have prevented anyway (can't have an item selected which hasn't been added yet). OTOH the autocomplete controller will itself adjust the index when it inserts new items in between, so nothing we'll have to care about ourselves.
Attachment #298032 - Flags: review?(seth)
Comment on attachment 298032 [details] [diff] [review] remove the offending code r=sspitzer
Attachment #298032 - Flags: review?(seth) → review+
thanks for taking care of this one, Simon.
Cool, thanks. I'll land this in just a sec.
Status: NEW → ASSIGNED
Checking in toolkit/content/widgets/autocomplete.xml; /cvsroot/mozilla/toolkit/content/widgets/autocomplete.xml,v <-- autocomplete.xml new revision: 1.107; previous revision: 1.106 done
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
Flags: in-litmus? → in-litmus+
You need to log in before you can comment on or make changes to this bug.