1551179 - Use SearchUtils.BROWSER_SEARCH_PREF in search tests

Status: RESOLVED FIXED

(Firefox :: Search, task, P3)

Description

[Mark Banner (:standard8)]

In bug 1550599 I added SearchUtils.jsm with BROWSER_SEARCH_PREF.

BROWSER_SEARCH_PREF is currently used in various places within our search tests, and we should replace the separate definitions with direct references to SearchUtils.BROWSER_SEARCH_PREF. Here's the current list of instances:

https://searchfox.org/mozilla-central/search?q=BROWSER_SEARCH_PREF&case=false&regexp=false&path=search%2Ftest

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.
   • Examine the existing source code lines
   • Remove the existing const BROWSER_SEARCH_PREF = "browser.search."; lines.
   • In toolkit/components/search/tests/xpcshell/head_search.js extend XPCOMUtils.defineLazyModuleGetters to reference SearchUtils.jsm.
   • In browser/components/search/test/browser/head.js please create a new XPCOMUtils.defineLazyModuleGetters call and move the existing CustomizableUITestUtils.jsm import into it, and add the import of SearchUtils.jsm
   • Replace the remaining references to BROWSER_SEARCH_PREF with SearchUtils.BROWSER_SEARCH_PREF
4. Check you changes for adherence to our guidelines by using:
   • ./mach lint toolkit/components/search browser/components/search
5. Build your change with mach build and test your change with:
   • ./mach xpcshell-test toolkit/components/search
   • ./mach mochitest browser/components/search
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 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 1

[premk]

Hi :standard8,
I'm new here. I'd like to help with this bug. Please assign it to me.

Comment 2

[Karan Sapolia]

Hello :standard8,
I would like to work on this too.

Comment 3

[Mark Banner (:standard8)]

premk, as you commented first I'll assign it to you. Note, please use moz-phab for submitting patches, it makes it easier for us.

Karan, I have some other options you may be interested in - I'll contact you direct.

Comment 4

[premk]

Thanks :standard8.

What is the resource reference path for importing SearchUtils.jsm using XPCOMUtils.defineLazyModuleGetters?

I tried poking around but couldn't find it.

Comment 5

[Mark Banner (:standard8)]

(In reply to premk from comment #4)

What is the resource reference path for importing SearchUtils.jsm using XPCOMUtils.defineLazyModuleGetters?

I tried poking around but couldn't find it.

The easiest way of finding references like this is to use searchfox, e.g. https://searchfox.org/mozilla-central/search?q=SearchUtils.jsm&case=false&regexp=false&path=

In this case you can copy https://searchfox.org/mozilla-central/rev/9eb2f739c165b4e294929f7b99fbb4d90f8a396b/toolkit/components/search/SearchEngine.jsm#13

Comment 6

[premk]

Attachment: Bug 1551179 - Use SearchUtils.BROWSER_SEARCH_PREF in search tests r=standard8

Comment 7

[premk]

Thanks, got it. Necessary changes made. All the tests passed.
I've submitted the patch to phabricator for your review.

Comment 8

[Mark Banner (:standard8)]

Hi, thank you for the patch. No need to needinfo here, Phabricator informs me automatically (as long as you've set me as reviewer).

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d8cc60b60e82
Use SearchUtils.BROWSER_SEARCH_PREF in search tests r=Standard8

Comment 10

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/d8cc60b60e82