1553236 - Change unit test callers for nsISearchService.addEngineWithDetails to use object passing rather than individual parameters

Status: RESOLVED FIXED

(Firefox :: Search, task, P3)

Description

[Mark Banner (:standard8)]

nsISearchService.addEngineWithDetails currently supports two formats of parameters:

Services.search.addEngineWithDetails(name, iconURL, alias, description, method, template, extensionID);

or

Services.search.addEngineWithDetails(name, details);

We want to migrate to using just the name, details form. To start off with, lets do just the unit tests, here's a link to the current places this affects:

https://searchfox.org/mozilla-central/search?q=search.addEngineWithDetails&case=false&regexp=false&path=test

Comment 1

[Mark Banner (:standard8)]

I'm opening this up as mentored. To help Mozilla out with this bug, here's the steps:

1. Comment here on the bug that you want to volunteer to help. I (or someone else) will assign it to you.
2. Download and build the Firefox source code: https://developer.mozilla.org/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build (an artifact build is sufficient).
   • If you have any problems, please ask on IRC (https://wiki.mozilla.org/Irc) in the #introduction channel. They're there to help you get started.
     You can also read the Developer Guide, which has answers to most development questions: https://developer.mozilla.org/docs/Mozilla/Developer_guide/Introduction
3. Start working on this bug.
   • See the link in comment 0 for the places to change. We want to change calls such as

await Services.search.addEngineWithDetails("MozSearch", "", "mozalias", "", "GET",
  "http://example.com/?q={searchTerms}");

to

await Services.search.addEngineWithDetails("MozSearch", {
  alias: "mozalias",
  method: "GET",
  template: "http://example.com/?q={searchTerms}",
});

* Note: you'll need to use different properties in the object depending on what the original call was. See the [documentation here](https://searchfox.org/mozilla-central/rev/6c9f60f8cc064a1005cd8141ecd526578ae9da7a/toolkit/components/search/nsISearchService.idl#298-360) for the possible parameter names.
* If a parameter is "" or null, then it doesn't need specifying in the object.
* If you have direct questions about this bug, feel free to ask here or on irc.

4. Check your changes for adherence to our guidelines by using:
   • ./mach lint --workdir
     • (this checks the uncommited changes in your work directory)
5. Build your change with mach build and test your change with:
   • ./mach test toolkit/components/search
   • You can also name the other directories where there are files that you're changing, but we'll also run these against our test servers once a patch has been posted to make sure nothing has broken.
6. Submit the patch for review. Mark me as a reviewer (r?standard8) so I'll get an email to come look at your code.
   • Here's the guide.
   • I strongly suggest using moz_phab to push the patch.
7. After a series of reviews and changes to your patch, I'll mark it for checkin or push it to autoland. Your code will soon be shipping to Firefox users worldwide!
8. ...now you get to think about what kind of bug you'd like to work on next. Let me know what you're interested in and I can help you find your next contribution.

Comment 2

[MELVIN GEORGE]

I would like to work on this issue.

Comment 3

[Mark Banner (:standard8)]

Welcome, please start working on it, and let us know if you get any issues.

Comment 4

[MELVIN GEORGE]

can you please point out the repository i need to start working on.

Comment 5

[Mark Banner (:standard8)]

(In reply to melvingeorge10 from comment #4)

can you please point out the repository i need to start working on.

If you follow the link in step 1 (i.e. https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build), pick the platform that you're starting on.

Then near the top of the page, it'll tell you what you need to do to get set up, and what commands you need to run to clone the repository (Generally in a "Get the source"/"Get the code" section).

Comment 6

[MELVIN GEORGE]

i need only mozilla-central repo.. but the script downloads mozilla-unified repo

Comment 7

[Mark Banner (:standard8)]

Unified is a superset of mozilla-central, so it doesn't matter except it is larger. If you want just mozilla-central, then use:

hg clone https://hg.mozilla.org/mozilla-central

Comment 8

[MELVIN GEORGE]

ok

Comment 9

[MELVIN GEORGE]

Attachment: Bug 1553236 - Change tests using Services.search.addEngineWithDetails() to use the object parameter form. r=standard8

Comment 10

[MELVIN GEORGE]

Attachment: Bug 1553236 - made some minor formatting fixes r=standard8

Depends on D32565

Comment 11

[MELVIN GEORGE]

Made all the changes to initial revision. ( D32565 - https://phabricator.services.mozilla.com/D32565 )

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/365c849f1085
Change tests using Services.search.addEngineWithDetails() to use the object parameter form. r=Standard8

Comment 13

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/365c849f1085