Pressing Enter to select tag from autocomplete closes bookmarks properties dialog

VERIFIED FIXED in Firefox 3.6a1

Status

()

defect
P3
normal
VERIFIED FIXED
11 years ago
10 years ago

People

(Reporter: whimboo, Assigned: mak)

Tracking

({verified1.9.1})

Trunk
Firefox 3.6a1
Points:
---
Dependency tree / graph
Bug Flags:
wanted-firefox3.5 +
in-testsuite +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081101 Minefield/3.1b2pre ID:20081101020236

If you open the bookmarks properties dialog for a bookmark and try to add some tags, you have to hit enter to select the tag from the autocomplete list. Doing so will close the whole dialog immediately. That shouldn't happen. Instead the tag has to be completed.

Steps to reproduce:
1. Add a bookmark by clicking the star and enter a tag e.g. "bug"
2. Open the bookmarks sidebar and select properties from the newly added bookmark
3. Go to the tags field and remove the tag "bug"
4. Press "b" to show up the autocomplete list - "bug" will be selected
5. Hit Enter

After step 5 the bookmarks properties dialog should not close as what it does actually.
No longer blocks: 415960
Blocks: 415960
Perhaps there should be one key (Enter?) to select+close and another (Tab?) to just select a tab from the dropdown list (and perhaps add a comma+<space>) but not close the dialog.

That way users can do both quickly: 
1. enter one tab and be done (via Enter).
2. enter multiple tags (via Tab) and be done (via Enter).
I don't think so that Enter should close the whole dialog. See for example other autocomplete boxes, e.g. location bar. You have to hit Enter twice. And this also makes sense.

Marco, do you have time for that bug? It can only be seen in the new bookmarks dialog. Within the bookmark panel it works fine.
Target Milestone: --- → Firefox 3.1
The behavior should be consistent with the other autocomplete impls in the browser.
Flags: wanted-firefox3.1+
Priority: -- → P3
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Posted patch patchSplinter Review
this fixes:
- pressing enter on any autocomplete popup in bookmarks panels won't submit (we actually have only tag autocomplete, but there's no reason to not be more generic)
- pressing enter on expanders should not submit
- moved listeners code to a common handleEvent to make code cleaner (i should have done this from the beginning really)
Attachment #357659 - Flags: review?(dietrich)
Attachment #357659 - Flags: review?(dietrich) → review+
Comment on attachment 357659 [details] [diff] [review]
patch

looks good, thanks for the cleanup, r=me.
http://hg.mozilla.org/mozilla-central/rev/cd75a236d62c
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: Firefox 3.1 → Firefox 3.2a1
Comment on attachment 357659 [details] [diff] [review]
patch

asking approval for this polish and usability bug
Attachment #357659 - Flags: approval1.9.1?
Verified with:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090123 Minefield/3.2a1pre (.NET CLR 3.5.30729) ID:20090123033224

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090123 Minefield/3.2a1pre

Markus, is it ok that we don't react on Enter while having the focus on the expand/collapse buttons? In other applications you will hear a 'dong' that this key is not valid. For now we do nothing.
Flags: in-litmus?
Status: RESOLVED → VERIFIED
Test case https://litmus.mozilla.org/show_test.cgi?id=7468 has been updated (step 5 was added) to reflect regression testing for this bug.
Flags: in-litmus? → in-litmus+
(In reply to comment #9)
> Test case https://litmus.mozilla.org/show_test.cgi?id=7468 has been updated
> (step 5 was added) to reflect regression testing for this bug.

I've disabled the test until the fix is checked into 1.9.1 branch.
Depends on: 475529
Whiteboard: [don't land on 1.9.1 unless Bug 475529 has approval]
Depends on: 485458
a test is being added in bug 485458
Flags: in-testsuite?
Flags: in-testsuite? → in-testsuite+
Aakash, can you please remove the Litmus test? With the automated test landed on 1.9.1 too we don't have to test this manually anymore. Thanks.
Flags: in-litmus+ → in-litmus?
Litmus test case 7468 has been deleted.
Flags: in-litmus? → in-litmus-
Comment on attachment 357659 [details] [diff] [review]
patch

a191=beltzner
Attachment #357659 - Flags: approval1.9.1? → approval1.9.1+
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a1f1d53aae30
Keywords: fixed1.9.1
Whiteboard: [don't land on 1.9.1 unless Bug 475529 has approval]
Verified fixed on 1.9.1 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090416 Shiretoko/3.5b4pre ID:20090416030924 and an appropriate build on WinXP.
Depends on: 491221
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.