karussell/jsii

Search is broken in chrome for indexes larger than 10.

Closed this issue · 10 comments

First, thank you for building this. I realise that you don't maintain this anymore, but I thought you might enjoy this bug.

The following unit test fails only in Chrome.

it("empty searches on indexes larger the 10 should work in chrome", function() {
    var jsii = new JSii();
    jsii.feedDocs([{
            "id": 1,
            "text": "s"
        }, {
            "id": 2,
            "text": "x"
        }, {
            "id": 3,
            "text": "x"
        }, {
            "id": 4,
            "text": "x"
        }, {
            "id": 5,
            "text": "x"
        }, {
            "id": 6,
            "text": "x"
        }, {
            "id": 7,
            "text": "x"
        }, {
            "id": 8,
            "text": "x"
        }, {
            "id": 9,
            "text": "x"
        }, {
            "id": 10,
            "text": "x"
        }, { //remove this object and the index works as expected
            "id": 11,
            "text": "x"
        }
    ]);
    var res = jsii.search("s");
    expect(res.docs[0].id).toBe(1); //passes

    jsii.search("");
    res = jsii.search("s");
    expect(res.docs[0].id).toBe(1); //fails finds 6
});

hmmh, strange. and in other browser this works? what happens if you change the page size to something higher

I had the same thought. No affect.

I think it might be a V8 bug, but wanted to pass it by you to see if anything jumped out.

On Apr 15, 2013, at 7:30 AM, Peter notifications@github.com wrote:

hmmh, strange. and in other browser this works? what happens if you change the page size to something higher


Reply to this email directly or view it on GitHub.

I don't think this is a V8 bug but I won't have the time to digg into this :(
Again: for other browsers this works?

BTW: why is jsii.search("") necessary :) ?

I have tested on OS X:

Safari,
Firefox
Opera
Chrome

It only fails on Chrome.

On Apr 15, 2013, at 7:34 AM, Peter notifications@github.com wrote:

I don't think this is a V8 bug but I won't have the time to digg into this :(
Again: for other browsers this works?


Reply to this email directly or view it on GitHub.

So it has something to do with the sort function.

If I comment out:

(around line 174 of jsii.js)

resDocs.sort(sortFunction);

It works.

On Apr 15, 2013, at 7:38 AM, Thomas Marsh ts.marsh@gmail.com wrote:

I have tested on OS X:

Safari,
Firefox
Opera
Chrome

It only fails on Chrome.

On Apr 15, 2013, at 7:34 AM, Peter notifications@github.com wrote:

I don't think this is a V8 bug but I won't have the time to digg into this :(
Again: for other browsers this works?


Reply to this email directly or view it on GitHub.

Oh, that it works in all other browsers is interesting :)

Probably related to http://stackoverflow.com/questions/3195941/sorting-an-array-of-objects-in-chrome ?

This issue would explain the "10": http://code.google.com/p/v8/issues/detail?id=90

"Sort is stable for length <= 10, unstable for length > 10: Chrome 15..17"

Looks like it.

When I make sort work on id rather than score it begins to behave correctly.

I'm going to add a score of zero to all docs whose score is undefined. I think that should fix it. I'll submit a pull request once I'm satisfied.

On Apr 15, 2013, at 7:52 AM, Peter notifications@github.com wrote:

Probably related to http://stackoverflow.com/questions/3195941/sorting-an-array-of-objects-in-chrome ?


Reply to this email directly or view it on GitHub.

Thanks a lot!

Fixed by pull request #2