Created attachment 253514 [details] [review]
SearchDisplay: handle certain result IDs specially

This was deferred because it was meant for epiphany, and the search provider was not merged at that time, but now it's in, so we should land this one too.

Review of attachment 253514 [details] [review]:

::: js/ui/remoteSearch.js
@@ +217,3 @@
+
+        let regularResults = results.filter(function(r) { return !_startsWith(r, 'special:') });
+        let specialResults = results.filter(function(r) { return _startsWith(r, 'special:'); });

Why a custom function instead of r.startsWith('special')?

@@ +219,3 @@
+        let specialResults = results.filter(function(r) { return _startsWith(r, 'special:'); });
+
+        return regularResults.slice(0, maxNumber).concat(specialResults);

So if I think my results are more important than anything, I can just mark all 50 of them as "special" and force them to be displayed? I'd rather see this as

let numSpecials = Math.min(specialResults.length, maxNumber);
let numRegulars = Math.min(regularResults.length - numSpecials, maxNumber);
return regularResults.slice(0, numRegulars).concat(specialResults.slice(0, numSpecials));

(In reply to comment #3)
> Review of attachment 253514 [details] [review]:
> @@ +219,3 @@
> +        let specialResults = results.filter(function(r) { return
> _startsWith(r, 'special:'); });
> +
> +        return regularResults.slice(0, maxNumber).concat(specialResults);
> 
> So if I think my results are more important than anything, I can just mark all
> 50 of them as "special" and force them to be displayed? I'd rather see this as
> 
> let numSpecials = Math.min(specialResults.length, maxNumber);
> let numRegulars = Math.min(regularResults.length - numSpecials, maxNumber);
> return regularResults.slice(0, numRegulars).concat(specialResults.slice(0,
> numSpecials));

No. If you mark all 50 results as special, then you are a bad application author, you will get bug reports and if you don't fix them people will stop using your search provider.
The design calls for 3 regular results and the special one, not 2 + 1.

Created attachment 257181 [details] [review]
SearchDisplay: handle certain result IDs specially

Allow the prefix 'special:' applied to result IDs to mark results
that should be always shown, even when they would overflow the
maximum results cap. This will be used by epiphany for the special
"Search the Web" result.

(In reply to comment #4)
> 
> No. If you mark all 50 results as special, then you are a bad application
> author

Read this as: "how much want we allow bad application authors to fuck up?" then. We also expect app authors to choose reasonably catchy names, but if they pick "My Supercalifragilisticexpialidocious App" anyway, we deal with it by ellipsizing the name ...


> The design calls for 3 regular results and the special one, not 2 + 1.

So we only expect a single special result? If so, we should enforce that, if not, I'd still prefer some limit (just reusing the normal one being the least arbitrary I suppose, resulting in a max of 3+3)

Created attachment 257186 [details] [review]
SearchDisplay: handle certain result IDs specially

Allow the prefix 'special:' applied to result IDs to mark results
that should be always shown, even when they would overflow the
maximum results cap. This will be used by epiphany for the special
"Search the Web" result.

Review of attachment 257186 [details] [review]:

OK.

Attachment 257186 [details] pushed as 002afda - SearchDisplay: handle certain result IDs specially