Awesomebar search results don't disappear when a search query that initially showed results doesn't yield results anymore

VERIFIED FIXED in Firefox 35

Status

()

Toolkit
Places
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: ttaubert, Assigned: ttaubert)

Tracking

Trunk
mozilla35
Points:
---
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox35 verified, firefox36 verified)

Details

Attachments

(1 attachment)

STR:

1) Type "bugzilla" in the Awesomebar assuming that you went to Bugzilla before.
2) Wait until some Awesomebar results are shown.
3) Now type " foo bar" so that "bugzilla foo bar" is the search term.

The old results should now disappear but instead they will stay. By disabling unified autocomplete the old behavior will be restored.
Can't reproduce this in my trunk profile, hm.
In my main profile, whenever I hit this, I see three occurrences of:

"Error(s) encountered during statement execution. Sqlite.jsm:618"
This seems to be caused by the MATCH_ANYWHERE search in case we don't get enough search results. Investigating further.
Hum. That fails for me because of the following SQLite error:

<database disk image is malformed>

Marco, any idea what to do about that? Should we handle those failures? If I wrap the conn.executeCached() calls in UnifiedComplete.js we at least resolve the promise returned by execute() instead of rejecting it and thus clear search results.
Flags: needinfo?(mak77)
Is your database really malformed, or is that error due to some unexpected error in the match function?
Could you please check the db with "PRAGMA integrity_check" ?

I guess we might indeed handle a failure differently here:

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/UnifiedComplete.js#1349

what we do currently is just reporting an error, we should probably invoke this.finishSearch(true) in both cases (success or failure), so we return whatever we got so far (and keep reporting error in the failure case).
Would this help in your case?

Something like:

this.getDatabaseHandle().then(conn => search.execute(conn))
                        .then(undefined, ex => { 
                          dump("Query failed: " + ex + "\n");
                          Cu.reportError(ex);
                        })
                        .then(() => {
                          if (search == this._currentSearch) {
                            this.finishSearch(true);
                        });

Though we should first figure the corruption type.
Flags: needinfo?(mak77)
(In reply to Marco Bonardo [::mak] (needinfo? me) from comment #5)
> Is your database really malformed, or is that error due to some unexpected
> error in the match function?
> Could you please check the db with "PRAGMA integrity_check" ?

wrong # of entries in index moz_places_guid_uniqueindex
wrong # of entries in index moz_places_url_uniqueindex
wrong # of entries in index moz_places_lastvisitdateindex
wrong # of entries in index moz_places_frecencyindex
wrong # of entries in index moz_places_visitcount
wrong # of entries in index moz_places_hostindex
wrong # of entries in index moz_places_faviconindex
rowid 242908 missing from index moz_historyvisits_dateindex
rowid 242908 missing from index moz_historyvisits_fromindex
rowid 242908 missing from index moz_historyvisits_placedateindex
wrong # of entries in index moz_historyvisits_dateindex
wrong # of entries in index moz_historyvisits_fromindex
wrong # of entries in index moz_historyvisits_placedateindex

:(
just issuing a REINDEX should fix it

now the funny part is that Places Maintenance we run weekly should have fixed this, but it didn't, that means likely your profile is not running idle-daily tasks?
that could be related to bug 973591...
(In reply to Marco Bonardo [::mak] (needinfo? me) from comment #8)
> that could be related to bug 973591...

Hah, just wrote a comment mentioning the exact same bug. Yeah, IRCCloud is always open. Should probably find out what's causing this.
(In reply to Marco Bonardo [::mak] (needinfo? me) from comment #5)
> this.getDatabaseHandle().then(conn => search.execute(conn))
>                         .then(undefined, ex => { 
>                           dump("Query failed: " + ex + "\n");
>                           Cu.reportError(ex);
>                         })
>                         .then(() => {
>                           if (search == this._currentSearch) {
>                             this.finishSearch(true);
>                         });

Tried and WFM.
(In reply to Marco Bonardo [::mak] (needinfo? me) from comment #7)
> just issuing a REINDEX should fix it

It did, thanks :)
Created attachment 8489872 [details] [diff] [review]
0001-Bug-1067085-Handle-Awesomebar-search-failures-better.patch

Well, that's pretty much what you suggested :)
Assignee: nobody → ttaubert
Status: NEW → ASSIGNED
Attachment #8489872 - Flags: review?(mak77)
Comment on attachment 8489872 [details] [diff] [review]
0001-Bug-1067085-Handle-Awesomebar-search-failures-better.patch

Review of attachment 8489872 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm!
Attachment #8489872 - Flags: review?(mak77) → review+
https://hg.mozilla.org/integration/fx-team/rev/8b581c66bded
https://hg.mozilla.org/mozilla-central/rev/8b581c66bded
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35

Updated

3 years ago
Flags: qe-verify+
Unfortunately, I was unable to replicate this issue on my end, several attempts were made on different test machines with the affected builds. 

Tim, is there any chance you could confirm this fix on your end?
Flags: needinfo?(ttaubert)
I can definitely say that I never saw this again since it was fixed. Is that enough to confirm? :)
Flags: needinfo?(ttaubert)
(In reply to Tim Taubert [:ttaubert] from comment #17)
> I can definitely say that I never saw this again since it was fixed. Is that
> enough to confirm? :)

I believe so, yes. Thank you, Tim! I'll keep an eye on this just in case.
Status: RESOLVED → VERIFIED
status-firefox35: --- → verified
status-firefox36: --- → verified
You need to log in before you can comment on or make changes to this bug.