Closed Bug 1551179 Opened 4 months ago Closed 3 months ago

Use SearchUtils.BROWSER_SEARCH_PREF in search tests

Categories

(Firefox :: Search, task, P3)

task

Tracking

()

RESOLVED FIXED
Firefox 69
Tracking Status
firefox69 --- fixed

People

(Reporter: standard8, Assigned: prem.kumar.krishnan, Mentored)

References

Details

(Keywords: good-first-bug, Whiteboard: [lang=js])

Attachments

(1 file)

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).
  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.
  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.

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

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

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.

Assignee: nobody → prem.kumar.krishnan

Thanks :standard8.

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

I tried poking around but couldn't find it.

Flags: needinfo?(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

Flags: needinfo?(standard8)

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

Flags: needinfo?(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).

Flags: needinfo?(standard8)
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d8cc60b60e82
Use SearchUtils.BROWSER_SEARCH_PREF in search tests r=Standard8
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 69
You need to log in before you can comment on or make changes to this bug.