Closed
Bug 1067085
Opened 10 years ago
Closed 10 years ago
Awesomebar search results don't disappear when a search query that initially showed results doesn't yield results anymore
Categories
(Toolkit :: Places, defect)
Toolkit
Places
Tracking
()
VERIFIED
FIXED
mozilla35
People
(Reporter: ttaubert, Assigned: ttaubert)
References
Details
Attachments
(1 file)
1.55 KB,
patch
|
mak
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•10 years ago
|
||
Can't reproduce this in my trunk profile, hm.
Assignee | ||
Comment 2•10 years ago
|
||
In my main profile, whenever I hit this, I see three occurrences of:
"Error(s) encountered during statement execution. Sqlite.jsm:618"
Assignee | ||
Comment 3•10 years ago
|
||
This seems to be caused by the MATCH_ANYWHERE search in case we don't get enough search results. Investigating further.
Assignee | ||
Comment 4•10 years ago
|
||
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)
Comment 5•10 years ago
|
||
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)
Assignee | ||
Comment 6•10 years ago
|
||
(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
:(
Comment 7•10 years ago
|
||
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?
Comment 8•10 years ago
|
||
that could be related to bug 973591...
Assignee | ||
Comment 9•10 years ago
|
||
(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.
Assignee | ||
Comment 10•10 years ago
|
||
(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.
Assignee | ||
Comment 11•10 years ago
|
||
(In reply to Marco Bonardo [::mak] (needinfo? me) from comment #7)
> just issuing a REINDEX should fix it
It did, thanks :)
Assignee | ||
Comment 12•10 years ago
|
||
Well, that's pretty much what you suggested :)
Comment 13•10 years ago
|
||
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+
Assignee | ||
Comment 14•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Updated•10 years ago
|
Flags: qe-verify+
Comment 16•10 years ago
|
||
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)
Assignee | ||
Comment 17•10 years ago
|
||
I can definitely say that I never saw this again since it was fixed. Is that enough to confirm? :)
Flags: needinfo?(ttaubert)
Comment 18•10 years ago
|
||
(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.
You need to log in
before you can comment on or make changes to this bug.
Description
•