1572488 - Test that the new configuration still gives the correct orders in various circumstances

Status: RESOLVED FIXED

(Firefox :: Search, task, P2)

Description

[Mark Banner (:standard8)]

Splitting this out from bug 1542243. I think we should add a few more tests for search engine orders, as I think there might be a bug lying around.

We should test things like (engine name -> order hint):

• A -> 750
• B -> 3000
• C -> 2000 Default = yes
• D -> 1000 Default = yes-if-no-other
• E -> 500 Default = yes-if-no-other.

Currently, I think this would come out as C, D, E, B, A, whereas I think it should be C, B, D, A, E.

Comment 1

[Mark Banner (:standard8)]

(we should do similar checks for private browsing engine as well).

Comment 2

[Dale Harvey (:daleharvey)]

Actually relooking probably best if you take this Mark. The engine selector does return C, D, E, B, A as that is what I would expect it to return.

Not sure why being a 'yes-if-no-other' engine wouldnt override the orderHint and thinking about the implementation it seems like it gets messy when handling multiple yes-if-no-others (same orderHint?), between having to handle default=yes|yes-if-no-other and private browsing this seemed like the simplest consistent solution that handled all cases (I would prefer the code be able to handle multiple default= yes / yes-if-no-others) but could be another way I am not thinking of

Comment 3

[Mark Banner (:standard8)]

(In reply to Mark Banner (:standard8) from comment #0)

• A -> 750
• B -> 3000
• C -> 2000 Default = yes
• D -> 1000 Default = yes-if-no-other
• E -> 500 Default = yes-if-no-other.

Currently, I think this would come out as C, D, E, B, A, whereas I think it should be C, B, D, A, E.

To explain this. Order Hint goes from highest to lowest. So if you ignore the default, this would be B, C, D, A, E. However, here the default is C, so that needs to be shifted first, this gives us C, B, D, A, E.

The issue with "yes-if-no-other" causes these engines to be a higher order, partially ignoring the order hint. That feels a bit wrong if we're prioritising the order hint.

I have a test written now, I'll see what I can figure out for the code.

Comment 4

[Mark Banner (:standard8)]

Attachment: Bug 1572488 - Improve handling of search engine ordering for the new SearchEngineSelector. r?daleharvey

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/45743b6fcc67
Improve handling of search engine ordering for the new SearchEngineSelector. r=daleharvey

Comment 6

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/45743b6fcc67