581076 - No "See all results" link present when searching for add-ons and not all are displayed (extensions.getAddons.maxResults)

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b2) Gecko/20100720 Firefox/4.0b2

Search for add-ons will return a list of found add-ons from AMO. If the number exceeds the limit (set by extensions.getAddons.maxResults), there is no link shown, the user can click to get forwarded to AMO. Users will be stuck and don't know where they can get the wanted add-on if it isn't present in the current list.

Steps:
1. Open about:config and set i.e. set extensions.getAddons.maxResults=4
2. Search for "url"

After step 2 the expected 4 results should be displayed but it's hard to get to the real list of all found add-ons. Right now, you will have to go to AMO manually and start a search again.

Comment 1

[Dave Townsend [:mossop]]

Boriss needs to look at this, don't think it will be a blocker though.

Comment 2

[Jennifer Morrow [:Boriss] (UX)]

Attachment: sketch: see all results

Actually, I'd call this blocking.  The link between AMO and the add-ons manager isn't one most users will be aware of, so failing to locate an add-on in the manager means the user might:

1. Give up.  "This add-on must not exist."
2. Keep trying different searches, which may eventually lead to the desired add-on.  Now the user knows the list of results is incomplete and that trying different combinations of black magic strings may actually produce the desired result.  

Instead, if the user has a link to AMO, they can get the full list of results,  hopefully find the add-on they want more easily, and can explore AMO if they'd like to search for a type of add-on rather than one in particular.

This link could just be the final item in the list of results (see attached)

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #2)
> Created attachment 460306 [details]
> sketch: see all results

The author is a left over, right? AFAIK we cannot display it for search results.

> 2. Keep trying different searches, which may eventually lead to the desired
> add-on.  Now the user knows the list of results is incomplete and that trying
> different combinations of black magic strings may actually produce the desired
> result.  

Which can also be difficult because of bug 581103. Given search terms do not always show up the expected results right now.

> Instead, if the user has a link to AMO, they can get the full list of results, 
> hopefully find the add-on they want more easily, and can explore AMO if they'd
> like to search for a type of add-on rather than one in particular.

In Namoroka we also show the total count of search results behind that string. Do we also wanna have it in the new add-ons manager? I would vote for that, because we have the information and it will give the user a much more detailed view of what is available.

Comment 4

[Dave Townsend [:mossop]]

Needed by string freeze

Comment 5

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1

Comment 6

[Dave Townsend [:mossop]]

Comment on attachment 469762 [details] [diff] [review]
Patch v1

More context would be better.

>@@ -73,3 +73,6 @@ enableAddonTooltip=Enable this add-on
> enableAddonRestartRequiredTooltip=Enable this add-on (restart required)
> disableAddonTooltip=Disable this add-on
> disableAddonRestartRequiredTooltip=Disable this add-on (restart required)
>+
>+#LOCALIZATION NOTE (showAllSearchResults) %S is the total number of search results
>+showAllSearchResults=See all %S results

This needs to go through the plural form stuff.

>diff --git a/toolkit/mozapps/extensions/content/extensions.js b/toolkit/mozapps/extensions/content/extensions.js

>         if (aIsRemote)
>           gCachedAddons[aObj.id] = aObj;
> 
>-        self._listBox.appendChild(item);
>+        self._listBox.insertBefore(item, self._listBox.lastChild);

insert before _allResultsLink

Comment 7

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #6)
> >+        self._listBox.insertBefore(item, self._listBox.lastChild);
> 
> insert before _allResultsLink

As discussed on IRC: These aren't actually the same nodes. So ignoring that.

Comment 8

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v2

Added PluralForm stuff. Fixed the themes - I had accidentally only changed winstripe. And discovered a bug where doing a search, switching views, and re-doing the same search would result in the link not showing. Fixed that (needed some refactoring) and added a test for it.

(In reply to comment #6)
> More context would be better.

Sorry, I thought I had that set to something better than it was.

Comment 9

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v2 - ready for check-in

Comment 10

[David Dahl :ddahl]

needs rebase:

anarchy ~/code/moz/mozilla-central/push-only/mozilla % hg qimport "https://bug581076.bugzilla.mozilla.org/attachment.cgi?id=470127" -n "see-all-results"
adding see-all-results to series file
anarchy ~/code/moz/mozilla-central/push-only/mozilla % hg qpush
applying see-all-results
patching file browser/app/profile/firefox.js
Hunk #1 FAILED at 50
1 out of 1 hunks FAILED -- saving rejects to file browser/app/profile/firefox.js.rej
patching file toolkit/mozapps/extensions/content/extensions.js
Hunk #2 FAILED at 1127
1 out of 6 hunks FAILED -- saving rejects to file toolkit/mozapps/extensions/content/extensions.js.rej
patching file toolkit/mozapps/extensions/content/extensions.xul
Hunk #1 FAILED at 212
1 out of 1 hunks FAILED -- saving rejects to file toolkit/mozapps/extensions/content/extensions.xul.rej
patching file toolkit/mozapps/extensions/test/browser/Makefile.in
Hunk #1 succeeded at 56 with fuzz 1 (offset 1 lines).
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh see-all-results

Comment 11

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v2.1 - ready for checkin

Rebased, and fixed the test to work again, after bug 562797 removed the callback to loadView()

Comment 12

[Blair McBride [:Unfocused] (UNAVAILABLE)]

https://hg.mozilla.org/mozilla-central/rev/6fe388a0fb5e

Comment 13

[Blair McBride [:Unfocused] (UNAVAILABLE)]

... and backed out due to test failures. test_AddonRepository.js was failing for some reason - no idea yet how it could do that.

Logs:
Linux opt: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1283144074.1283144875.30462.gz
OSX opt: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1283145114.1283146001.2227.gz

Comment 14

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Backout changeset was: 
https://hg.mozilla.org/mozilla-central/rev/0487454f4219

Comment 15

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Given I don't know why that test failed, and we're right up against the wire with the freeze - I would rather re-target this for beta6, and land soon after the freeze.

Comment 16

[Mike Beltzner [:beltzner, not reading bugmail]]

--> beta6+

Comment 17

[Axel Hecht [:Pike]]

Comment on attachment 470365 [details] [diff] [review]
Patch v2.1 - ready for checkin

"See all 1 result", really? I suspect that should rather be "See one result", right?

Comment 18

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #17)
> Comment on attachment 470365 [details] [diff] [review]
> Patch v2.1 - ready for checkin
> 
> "See all 1 result", really? I suspect that should rather be "See one result",
> right?

Eh, I guess. Technically, that case will never show - the number will always be greater than 1. The plural form was used only for other languages that special-case more than just "1".

Comment 19

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v2.2

Turned out test_AddonRepository.js was expecting extensions.getAddons.search.browserURL to not exist - and that in turn messed up various other parts of the test. Fix was just a simple switch of a boolean to tell it to expect it to exist. Carrying forward the r+, since that was such a trivial change.

Comment 20

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/164c156307c7

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+1]]

So the number of search results we display in the search pane is still different from the results found on AMO. But that's another bug which is already on file and I cannot find right now.

The link is displayed in builds on all platforms like Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b7pre) Gecko/20100922 Firefox/4.0b7pre

Comment 22

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #21)
> But that's another bug which is
> already on file and I cannot find right now.

Bug 593332 ?

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Could be. We will have to see.