2024714 - If there are duplicate keywords for search engines, prefer the default search engine

Status: RESOLVED FIXED

(Firefox :: Search, task, P2)

Description

[Mark Banner (:standard8)]

Although we generally try to prevent it, there are some cases where a search engine could be added with a keyword that's a duplicate of one of the application provided engines.

Currently, it depends on the visual order of search engines as to which one is used. We should make this more consistent, by using the search engine according to the following (in order):

• Use whichever is the default search engine, or
• Use whichever is application provided, or
• Use the first search engine in the sorted visible order.

Comment 1

[Mark Banner (:standard8)]

Attachment: Bug 2024714 - Change browser_tokenAlias test to use configuration based engines rather than manually added engines. r?#urlbar-reviewers!

Comment 2

[Mark Banner (:standard8)]

Attachment: Bug 2024714 - When handling duplicate keywords, use a predictable order. r?#urlbar-reviewers!

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/57974752dd76
https://hg.mozilla.org/integration/autoland/rev/c7e6beeda02f
Change browser_tokenAlias test to use configuration based engines rather than manually added engines. r=urlbar-reviewers,jteow
https://github.com/mozilla-firefox/firefox/commit/03d7a6bb9b0b
https://hg.mozilla.org/integration/autoland/rev/e7cf9d0f4c39
When handling duplicate keywords, use a predictable order. r=urlbar-reviewers,jteow

Comment 4

[Mark Banner (:standard8)]

Attachment: Bug 2024714 - Change browser_tokenAlias test to use configuration based engines rather than manually added engines.

Original Revision: https://phabricator.services.mozilla.com/D289674

Comment 5

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined/Reason for urgency: In 150 we are planning on some changes which may introduce scenarios with duplicate keywords. We want to ensure they are correctly handled, for consistency from the user's perspective and for correct attributions.
• Code covered by automated testing?: yes
• Fix verified in Nightly?: no
• Needs manual QE testing?: yes
• Steps to reproduce for manual QE testing: Test the scenarios from comment 0 on the bug where there's an add-on installed with a keyword that's a duplicate of an application provided engine.

The search engineering team can help set up the config.

• Risk associated with taking this patch: low
• Explanation of risk level: Small concentrated change to how keywords are processed, in well tested code.
• String changes made/needed?: None
• Is Android affected?: no

Comment 6

[Mark Banner (:standard8)]

Attachment: Bug 2024714 - When handling duplicate keywords, use a predictable order.

Original Revision: https://phabricator.services.mozilla.com/D289675

Comment 7

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/c7e6beeda02f
https://hg.mozilla.org/mozilla-central/rev/e7cf9d0f4c39

Comment 8

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/dad2557c2549
https://hg.mozilla.org/releases/mozilla-beta/rev/49ba76352e26
https://github.com/mozilla-firefox/firefox/commit/1cb1194b6f91
https://hg.mozilla.org/releases/mozilla-beta/rev/b28d393ce4e6

Comment 9

[Cristian Baica [:cbaica], Release Desktop QA]

Hey Mandy, could you help us with the config setup in order to test this? Also, any pointers are highly appreciated.

Comment 10

[Mark Banner (:standard8)]

Gave details on Slack - we have an example on staging that can be used with an add-on to verify.

Comment 11

[Adrian Florinescu [:aflorinescu]]

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

Gave details on Slack - we have an example on staging that can be used with an add-on to verify.

Initial testing completed, I gave details on slack related concerns related to this fix, will further update this issue as soon as discussion there concludes.