Closed Bug 720066 Opened 8 years ago Closed 8 years ago

Tagging broken, cannot type in the tag field

Categories

(Toolkit :: Autocomplete, defect)

12 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla12
Tracking Status
firefox12 - ---

People

(Reporter: ancestor.ak, Assigned: mak)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

Typing works fine as long as there are autocomplete results from your existing tags. As soon as there are no matches, the field gets cleared. I believe it may be caused by the recent autocomplete changes.

Works: http://hg.mozilla.org/mozilla-central/rev/58e933465c36
Broken: http://hg.mozilla.org/mozilla-central/rev/5c2bc94d359c

Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=58e933465c36&tochange=5c2bc94d359c
Blocks: 566489
taking to not lose it, but feel free to steal it.
Assignee: nobody → mak77
Tags Field of Bookmark Properties dialog , Edit Bookmark panel and and Library window are also broken.

Regression window(m-c)
Works:
http://hg.mozilla.org/integration/mozilla-inbound/rev/eea95e86541f
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0a1) Gecko/20120119 Firefox/12.0a1 ID:20120119024012
Fails:
http://hg.mozilla.org/integration/mozilla-inbound/rev/19a5e75b8ed8
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0a1) Gecko/20120119 Firefox/12.0a1 ID:20120119033312
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=eea95e86541f&tochange=19a5e75b8ed8

Triggered by:
51dcdc85081c	Michael Ventnor — Bug 660592 - Allow autocomplete results to hide themselves from the popup. r=mak sr=gavin
Assignee: mak77 → nobody
Blocks: 660592
Component: Bookmarks & History → Autocomplete
OS: Windows 7 → All
Product: Firefox → Toolkit
QA Contact: bookmarks → autocomplete
Hardware: x86_64 → x86
Version: Trunk → 12 Branch
Duplicate of this bug: 720133
had the same problem again after removing some addons and restarting the browser
Duplicate of this bug: 720166
Duplicate of this bug: 720197
Hardware: x86 → All
Duplicate of this bug: 720131
taking to not lose it from my radar, though feel free to steal it if you have a fix.
Assignee: nobody → mak77
So the patch removed a mRowCount == 0 bailout from CompleteDefaultIndex(), cause mRowCount does not take into account typeAheadResults. The problem is tags autocomplete result notifies RESULT_SUCCESS instead of RESULT_NOMATCH even if it has no results, so we actually go through CompleteDefaultIndex even if the complete value is an empty string. The previous check was protecting us from this kind of weirdness.
So I think to add back a protection from these cases (Actually adding a check that the index we try to complete is < than the number of matches), and fix the tags autocomplete implementation to return a proper message.
Duplicate of this bug: 720387
Attached patch patch v1.0 (obsolete) — Splinter Review
This would be the patch, looking into a test now, if possible.
Attached patch patch v1.1Splinter Review
Comes with a test now.
I was not sure if we would like to also add a fatal assertion for debug mode there, though it would make impossible to test the fix in debug mode. I may add a warning if you wish, though it will create noise in the test.
Attachment #590741 - Attachment is obsolete: true
Attachment #590766 - Flags: review?(gavin.sharp)
Status: NEW → ASSIGNED
Flags: in-testsuite+
Duplicate of this bug: 720640
Comment on attachment 590766 [details] [diff] [review]
patch v1.1

I was going to suggest putting this check in the loop above, but that'd be more complicated and wouldn't really make much of a difference in practice, so this is fine. This also made me find bug 720598...
Attachment #590766 - Flags: review?(gavin.sharp) → review+
follow up to fix a typo, I should not code in cpp and js at the same time, I end up confusing array types :)
https://hg.mozilla.org/integration/mozilla-inbound/rev/f35bd9404ee4
Duplicate of this bug: 721138
working great in today's nightly over here
well done!
You need to log in before you can comment on or make changes to this bug.