1624733 - Remove addParam from nsISearchEngine

Status: RESOLVED FIXED

(Firefox :: Search, task, P3)

Description

[Mark Banner (:standard8)]

nsISearchEngine.addParam is no-longer necessary in production code, so we should look at removing it from the idl.

We still use it in tests, but I think we should be able to provide a helper function in SearchTestUtils.jsm which creates the engine in one hit using addEngineWithDetails.

This would also help to simplify those tests a bit.

Comment 1

[Mark Banner (:standard8)]

Setting this as a mentored bug, as I think it is suitable, especially for someone who has done patches for Mozilla already.

Clarifying link for where this appears to be used in tests: https://searchfox.org/mozilla-central/search?q=engine.*addParam&case=false&regexp=true&path=*.js

As mentioned above, we should create a new method in SearchTestUtils, probably based on the existing promiseNewSearchEngine, but rather than taking the details, takes the same information as for addEngineWithDetails. We should be able to pass the param data via the details object, something like { postData: "q={searchTerms}" } (depending on the exact options addParams was originally called with).

We should also move the documentation from the nsISearchService file to the function in SearchEngine.jsm, and rename SearchEngine.addParams to SearchEngine._addParams.

If there's basic questions on how to do patches and things, then #introduction on Matrix is a good place to start, if there's questions about the details of the changes, I can be found as Standard8 in the #search channel (or just ask here).

Comment 2

[Mark Banner (:standard8)]

In working on this, I've just realised we don't need a separate function on SearchTestUtils - addEngineWithDetails already can take the necessary post/get parameters separately, so we just need to combine the previous addParam calls into it.

Comment 3

[Mark Banner (:standard8)]

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=25ec9aeea7a1e29b401aa43fef3ee5dcd114fe19

Comment 4

[Mark Banner (:standard8)]

Attachment: Bug 1624733 - Remove addParam from nsISearchEngine, as it is only used in tests. r?daleharvey!

Depends on D77653

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/731578a5ef95
Remove addParam from nsISearchEngine, as it is only used in tests. r=daleharvey

Comment 6

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/731578a5ef95