Search results change their order after pressing space or further typing

RESOLVED FIXED in mozilla1.9.3a1

Status

()

Toolkit
Places
--
minor
RESOLVED FIXED
10 years ago
8 years ago

People

(Reporter: Robert [minotaur 11], Assigned: khuey)

Tracking

({regression})

1.9.0 Branch
mozilla1.9.3a1
regression
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(status1.9.2 beta5-fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; de; rv:1.8.1.13) Gecko/20080325 Ubuntu/7.10 (gutsy) Firefox/2.0.0.13
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9pre) Gecko/2008033103 Minefield/3.0pre

Sometimes search results in the location bar dropdown list change their order after pressing space to enter a further search term. Not all search results are affected, this seems to happen only for entries with identical frecency. The behavior is especially noticeable if results on positon 1 to 6 (visible without scrolling) change their place with results on position 7 to 12.

Reproducible: Always

Steps to Reproduce:
1.Create two bookmarks: http://a1/, http://a2/
2.Type a
3.Press space
Actual Results:  
The order in step 2 is: a2, a1. After step 3: a1, a2.


Regression range:
20080322_2244_firefox-3.0b5pre.en-US.linux-i686.tar.bz2 works
20080322_2340_firefox-3.0b5pre.en-US.linux-i686.tar.bz2 fails

Bonsai query:
http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1206251040&maxdate=1206254399
(Reporter)

Updated

10 years ago
Keywords: regression
Version: unspecified → Trunk
(Reporter)

Comment 1

10 years ago
It's not restricted to pressing space, i've also seen this if previous search results with identical frecency are reused for the first time during further typing.

STR:
1.Create two bookmarks: http://aaa1/, http://aaa2/
2.Type a (a): aaa2, aaa1
3.Type a (aa): aaa1, aaa2
4.Type a (aaa): aaa1, aaa2
Summary: Search results change their order after pressing space → Search results change their order after pressing space or further typing

Comment 2

10 years ago
This is related to the adaptive learning. It probably learned that you previously selected "aaa2" when typing "a", but when you type "aa", it doesn't know anything about it, so it doesn't give adaptive learning preference to "aaa2" anymore.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 3

10 years ago
I deleted the inputhistory with "DELETE FROM moz_inputhistory;" before i tried to reproduce it. The regression range indicates that it is related to the incremental search updates.
(Reporter)

Comment 4

10 years ago
I tried it in a new profile again and it seems to depend on the order of the entries in the places.sqlite file. I only see it if i create first the bookmark aaa1 and then aaa2 (or a1 and a2).

Updated

10 years ago
Duplicate of this bug: 430679
Hardware: x86 → All
Created attachment 405972 [details] [diff] [review]
Make places query stable

This bug is caused by the fact that a sort in SQL is not guaranteed to be stable w/r/t fields not in the ORDER BY clause.  By using the primary key in the ORDER BY clause we can guarantee that the sort is stable w/r/t everything.  This fixes the non-deterministic behavior in the Location Bar seen in this bug.
Assignee: nobody → me
Status: NEW → ASSIGNED
Attachment #405972 - Flags: review?(dietrich)
Comment on attachment 405972 [details] [diff] [review]
Make places query stable

id should be fine for a tie-breaker, thanks for the fix! can you add a test, or update an existing test to cover this case, and re-request review?
Attachment #405972 - Flags: review?(dietrich) → review+
Can you attach a patch with more context too?
Flags: in-testsuite?
(In reply to comment #7)
> (From update of attachment 405972 [details] [diff] [review])
> id should be fine for a tie-breaker, thanks for the fix! can you add a test, or
> update an existing test to cover this case, and re-request review?

(In reply to comment #8)
> Can you attach a patch with more context too?

Yeah, no problem.
have you tested this against a big database to be sure it is not a perf killer like bug 412736?
Created attachment 406431 [details] [diff] [review]
Patch w/test

Addressing comments 7 and 8.
Attachment #405972 - Attachment is obsolete: true
Attachment #406431 - Flags: review?(dietrich)
(In reply to comment #10)
> have you tested this against a big database to be sure it is not a perf killer
> like bug 412736?

I have not tested it on a large database but, at least in theory, there is no reason why sorting by the primary key should have s significant impact on performance.  The reason that bug 412736 was a perf killer is because there are no indexes on the columns that were being used in that sort.  E.g.

EXPLAIN SELECT * FROM moz_places h ORDER BY h.frecency DESC
[28 opcodes returned]

EXPLAIN SELECT * FROM moz_places h ORDER BY h.frecency DESC, h.id DESC
[28 opcodes returned]

EXPLAIN SELECT * FROM moz_places h ORDER BY h.frecency DESC, h.typed DESC
[49 opcodes returned]
i was mostly concerned by the UNION, but being a union all should not make a difference. ok fine by me, the explain is quite clear.
Attachment #406431 - Flags: review?(dietrich) → review+
Comment on attachment 406431 [details] [diff] [review]
Patch w/test

r=me, thanks. sorry for long wait.
(In reply to comment #14)
> (From update of attachment 406431 [details] [diff] [review])
> r=me, thanks. sorry for long wait.

Thanks!
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/7f76978080a1
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Component: Location Bar and Autocomplete → Places
Flags: in-testsuite? → in-testsuite+
Keywords: checkin-needed
Product: Firefox → Toolkit
QA Contact: location.bar → places
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
Version: Trunk → 1.9.0 Branch
Comment on attachment 406431 [details] [diff] [review]
Patch w/test

i think the change is small enough to be considered for 1.9.2, giving back more consistent results is a polish win for the awesomebar.
Attachment #406431 - Flags: approval1.9.2?
Keywords: checkin-needed
Keywords: checkin-needed
Comment on attachment 406431 [details] [diff] [review]
Patch w/test

approval1.9.2 requests aren't currently being monitored, since we're nearing RC freeze and there are too many outstanding requests, so I'm clearing this request. Feel free to re-request approval if you are confident that it's worth drivers' time to consider whether this non-blocker needs to land for 1.9.2 at this stage.
Attachment #406431 - Flags: approval1.9.2?
Comment on attachment 406431 [details] [diff] [review]
Patch w/test

a192=beltzner, small change, big test.
Attachment #406431 - Flags: approval1.9.2+
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/b437bbc7f874
status1.9.2: --- → final-fixed
You need to log in before you can comment on or make changes to this bug.