As of Nightly 2014-11-25, keyword searches seem to drop the last few characters if I press enter very fast. I am not using e10s at the moment. In #fx-team, Unfocused suspected this is related to bug 1067903.

Yea, fallout from bug 1067903.

We detect if a keyword search matches via a synchronous match. But we don't actually add a match until we do an async DB query. We get the current sync match via a cache, and do the DB query to get the title - so we should just be able to also cache the title, and make it all sync. Longer term, we may wish to implement waiting for the first special result if it hasn't returned yet before the enter key has been pressed (not to block the enter key, but to wait on navigating). But that's tricky, and would rely on the controller rewrite. Thankfully, I don't think we need it yet.

is this about search aliases or bookmark keywords? since currently bookmarks keywords are no more a synchronous look up (there is at least one tick, the first query in the session will be slower to build the cache though) fwiw, there's no way we can do a DB query synchronously, we just shouldn't.

(In reply to Marco Bonardo [::mak] (needinfo? me) from comment #3) > is this about search aliases or bookmark keywords? > since currently bookmarks keywords are no more a synchronous look up (there > is at least one tick, the first query in the session will be slower to build > the cache though) > > fwiw, there's no way we can do a DB query synchronously, we just shouldn't. For me, it's about a bookmark with a keyword, whose URL also uses the "%s" parameter to let it also accept a query. So, I guess that means "bookmark keyword" (even though I use it for searching...). Hopefully I'm being specific enough. :)

[Tracking Requested - why for this release]: It's not just keyword searches that are broken. Typed URLs such as "about:crashes" also have their last few characters dropped.

Apparently Bugzilla doesn't want to submit what I type either. Filed bug 1105155.

(In reply to Jesse Ruderman from comment #5) > It's not just keyword searches that are broken. Typed URLs such as > "about:crashes" also have their last few characters dropped. yep, confirmed, and this is much worse than keywords.

The uplift is Friday. Can we please backout bug 1067903?

(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #8) > The uplift is Friday. Can we please backout bug 1067903? we will instead disable unifiedcomplete in Nightly until these bad recent regressions are handled, so we can continue working on top of the current code and not risk bitrotting and a dependencies nightmare. bug 1105456.

Not tracking - this is specific to UnifiedComplete, which was disabled via bug 1105456.

(In reply to Marco Bonardo [::mak] (needinfo? me) from comment #3) > fwiw, there's no way we can do a DB query synchronously, we just shouldn't. No, we don't need (or want!) a synchronous query here. We currently use the cache to only detect bookmark keywords, but the DB to fetch info about them - because the cache neither has all the info we want (it lacks the title), or the API. So we modify the cache so we can just use that, instead of a DB query each time.

Oh, and I'm treating bug 1105967 as separate. That's a generic issue with the controller/popup, whereas this bug has a specific fix in UnifiedComplete.

(In reply to Blair McBride [:Unfocused] from comment #13) > (In reply to Marco Bonardo [::mak] (needinfo? me) from comment #3) > > fwiw, there's no way we can do a DB query synchronously, we just shouldn't. > > No, we don't need (or want!) a synchronous query here. We currently use the > cache to only detect bookmark keywords, but the DB to fetch info about them > - because the cache neither has all the info we want (it lacks the title), > or the API. So we modify the cache so we can just use that, instead of a DB > query each time. I'd like to understand which code you intend to touch, cause I'm rewriting the keywords cache in bug 1083469 and at this point maybe we want a different kind of rewrite? (fwiw that bug is blocked by a fancy BC1 orange on e10s) The code that is in the tree currently is not what we will use, cause it's currently synchronous.

(In reply to Marco Bonardo [::mak] (needinfo? me) from comment #15) > The code that is in the tree currently is not what we will use, cause it's > currently synchronous. Heh, I was going to ask you about that, after I looked at that code again. I think your patch in bug 1083469 is what I was thinking of :) So, looking at that patch, it introduces more asynchronicity to the first result. So my idea here won't really help in the way intended. But it will help in that we can reduce a DB query (_keywordQuery). Given that, how about we de-prioritise this bug until sometime after bug 1083469 lands. In the mean time, I'll concentrate on bug 1105967.

(In reply to Blair McBride [:Unfocused] from comment #16) > Given that, how about we de-prioritise this bug until sometime after bug > 1083469 lands. In the mean time, I'll concentrate on bug 1105967. I think it's a good idea. As well as we can avoid keywordQuery by making the cache smarter, so, I could make bug 1083469 different. My issue is mostly the orange atm, but once that is somehow figured, I can make the code smarter.

Making this clearer, as summarized in comment 17.

Currently we don't query anymore, we use PlacesUtils.keywords.fetch now, that doesn't have to go through the db.

I'm marking this as WFM based on comment 19, the keywords API refactoring has basically already solved this.