560618 - Sorting search results by Name is broken

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Public Pages, defect, P3)

Description

[krupa raj[:krupa]]

Attachment: screenshot

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3

steps to reproduce:
1.Search for "fire"
2.Sort the search results by "Name"


observed behavior:
Add-ons are not arranged in alphabetical order.

Add-ons  with names starting with "E" are listed before add-ons with name starting with "B"

behavior can also be noticed at:
https://preview.addons.mozilla.org/z/en-US/firefox/search/?sort=name&pp=20&atype=0&pid=3&lver=any&q=search+for+add-ons&appid=1&cat=all&page=7&advanced=on

Comment 1

[Dave Dash [:davedash, :dd] (assign all bugs to mbrandt)]

There are two bugs here.

1. E before B.
2. Z before a.

The latter is sphinx not being case insensitive or folding cases.  I'll need to research why that's happening.

1. I have a hypothesis that I can't verify locally - if you can find similar examples that would help.  My theory is that there is a translation for those addons that happens to give it a better ranking score, but since we pull the addon out of the DB (rather than on the field sphinx used), we get this.

If my hypothesis is right with 1, I'll have to order by matching locale and then name.

I'll try to solve problem 2 first, since it's easy to reproduce.

Comment 2

[Dave Dash [:davedash, :dd] (assign all bugs to mbrandt)]

Attachment: Uppercasing name_ord so we can sort by it.

This is a pre-requisite for fixing this.  We need a lowercase field to search against.  Went with upper since ABC seems to come before a lot of punctuation like [ or ].

Also QA, this is not a regression, and should probably be NOT filed against the current milestone.

-d

Comment 3

[James Socol [:jsocol, :james]]

Comment on attachment 440348 [details] [diff] [review]
Uppercasing name_ord so we can sort by it.

This looks fine to me.

Comment 4

[Dave Dash [:davedash, :dd] (assign all bugs to mbrandt)]

http://github.com/jbalogh/zamboni/commits/ad95fed

QA note: If this isn't working right away, there may be an indexing issue with Sphinx, since we recently switch to pulling form github.  Don't reopen, just ask.

Comment 5

[krupa raj[:krupa]]

> http://github.com/jbalogh/zamboni/commits/ad95fed
> 
> QA note: If this isn't working right away, there may be an indexing issue with
> Sphinx, since we recently switch to pulling form github.  Don't reopen, just
> ask.


krupa: davedash: I'm reopening bug 560618.You asked QA to ping you before reopening...(In reply to comment #4)

Bug is still happening at https://preview.addons.mozilla.org/en-US/firefox/search/?sort=name&pp=20&atype=0&pid=1&lver=any&q=fire&cat=all&page=2&advanced=on

Comment 6

[Dave Dash [:davedash, :dd] (assign all bugs to mbrandt)]

We should wontfix this.  It's fixed locally for me, my guess is there's some data weirdness - but I don't think it's worth spending time to fix.

Mostly because sort by NAME is a worthless feature and we should get rid of it.

Comment 7

[Wil Clouser [:clouserw]]

We don't sort by name anywhere else that I know of.  I'd be fine removing the option.  Fligtar?

Comment 8

[Wil Clouser [:clouserw]]

(In reply to comment #7)
> We don't sort by name anywhere else that I know of.  I'd be fine removing the
> option.  Fligtar?

It looks like we also sort by name on Themes' category pages, but I'd be fine getting rid of it there too.

Comment 9

[Justin Scott [:fligtar]]

Name is the second most popular sort option (behind 'popular'). Would rather not remove based on that.

Comment 10

[Justin Scott [:fligtar]]

Correction. It's actually the second-most in terms of hits and most popular in terms of visits.

Comment 11

[Dave Dash [:davedash, :dd] (assign all bugs to mbrandt)]

I think we should be getting users to search better.  Sorting by name implies their search has failed.

Comment 12

[Wil Clouser [:clouserw]]

I talked to dd about this on IRC.  Sphinx doesn't have a way to sort by current locale, then add-on's default locale.  If we implemented that it would take a bunch of time and completely change our caching strategy.

This isn't going to make 5.11.  With the new info I'm more inclined to wontfix this and fix our search suggestions bug so people don't need to sort by name anymore.

Comment 13

[Justin Scott [:fligtar]]

If the most popular sorting method of browse pages is broken (or any method, really), I think that's a blocker for pushing them live.

We can consider alternate ideas, but I don't think removing this sort because it's hard or because you don't see value in it is viable based on its usage.

Comment 14

[Dave Dash [:davedash, :dd] (assign all bugs to mbrandt)]

Sorry Wil's comment and my suggestion isn't that we should remove it (for now), it's that we should WONTFIX this particular bug and let a few queries be sorted incorrectly.

Here's a sphinx catalog:

id | addon name | locale
1  | foo bar  | en
1  | bar de foo | fr
2 | apple | en
3 | cat | en

and it's told to sort  by name:

we get:
2, 1, 3

apple
foo bar
cat

So our options are:

Pull out non matching locales and then sort:

apple
cat
foo bar

but then if we are fr we'd only get:

bar de foo

or we pull everything out (up to 1000 results) and sort them manually in python:

this makes sorting, paginating, and  everything else very difficult, and search less stable.

We could compromise and just sort in the page you're on, e.g. everything on a given page will be in the correct order, but that's a fix that doesn't really fix the problem.


As far as blockiness is concerned, this is an issue we already have in production:

https://addons.mozilla.org/en-US/firefox/search?sort=name&q=bar%20pdf


The reason you've never noticed is because if you did on the off chance decide you needed alphabetical order as a sort, *most* items sorted, because most items sort well across locales in our app.

In other words, this effects few people - even if sort by name.

Comment 15

[Justin Scott [:fligtar]]

Ok. When sorting by name, let's only include add-ons with the current locale and add a note at the top: http://grab.by/4AUQ

Comment 16

[Dave Dash [:davedash, :dd] (assign all bugs to mbrandt)]

WIN!

Comment 17

[krupa raj[:krupa]]

(In reply to comment #15)
> Ok. When sorting by name, let's only include add-ons with the current locale
> and add a note at the top: http://grab.by/4AUQ

Will all our sort types be locale-specific then?

Comment 18

[Dave Dash [:davedash, :dd] (assign all bugs to mbrandt)]

No, those work fine as they are.

Comment 19

[Dave Dash [:davedash, :dd] (assign all bugs to mbrandt)]

http://github.com/davedash/zamboni/commit/129ddac5856e6d48f6e4a359065bb8e7adc38ebc

Comment 20

[Dave Dash [:davedash, :dd] (assign all bugs to mbrandt)]

http://github.com/jbalogh/zamboni/commits/02d3792