Last Comment Bug 839670 - Remove usage of gXULAppInfo and createAppInfo in toolkit/components/search/tests/xpcshell/
: Remove usage of gXULAppInfo and createAppInfo in toolkit/components/search/te...
Status: RESOLVED FIXED
[mentor=MattN][lang=js]
:
Product: Firefox
Classification: Client Software
Component: Search (show other bugs)
: Trunk
: All All
: -- minor (vote)
: Firefox 21
Assigned To: Joshua Kalpin
:
Mentors:
http://dxr.allizom.org/search?tree=mo...
Depends on: 809920
Blocks:
  Show dependency treegraph
 
Reported: 2013-02-08 14:45 PST by Matthew N. [:MattN]
Modified: 2013-02-15 13:38 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
First patch to remove createAppInfo (6.91 KB, patch)
2013-02-14 19:25 PST, Joshua Kalpin
MattN+bmo: review+
Details | Diff | Splinter Review
Second patch (7.55 KB, patch)
2013-02-15 06:28 PST, Joshua Kalpin
no flags Details | Diff | Splinter Review
Actual second patch (7.55 KB, patch)
2013-02-15 06:29 PST, Joshua Kalpin
no flags Details | Diff | Splinter Review

Description Matthew N. [:MattN] 2013-02-08 14:45:55 PST
Since bug 809920 landed, we can use updateAppInfo from AppInfo.jsm[1].

[1] https://developer.mozilla.org/en-US/docs/Using_nsIXULAppInfo#Getting_nsIXULAppInfo_in_xpcshell_tests
Comment 1 Joshua Kalpin 2013-02-12 19:29:13 PST
First bug for me but I think I should be able to handle it if it hasn't been claimed yet.
Comment 2 Matthew N. [:MattN] 2013-02-12 19:37:10 PST
Excellent! Let me know if you have any questions.
Comment 3 Joshua Kalpin 2013-02-14 19:25:46 PST
Created attachment 714239 [details] [diff] [review]
First patch to remove createAppInfo

First attempt at the replacement. Not 100% sure if I did it quite right.
Comment 4 Matthew N. [:MattN] 2013-02-14 22:51:04 PST
Comment on attachment 714239 [details] [diff] [review]
First patch to remove createAppInfo

Review of attachment 714239 [details] [diff] [review]:
-----------------------------------------------------------------

Excellent work on your first patch! Only a few comments to address below.

Could you update your .hgrc file to include the username field so that it gets included in the commit message? See https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Once you make these changes, go ahead an upload another version in this bug and set the checkin flag to '?' and I will push the patch.

Thanks!

::: toolkit/components/search/tests/xpcshell/head_search.js
@@ -14,1 @@
>  const BROWSER_SEARCH_PREF = "browser.search.";

Could you also remove the definitions of XULAPPINFO_CONTRACTID and XULAPPINFO_CID above this?

::: toolkit/components/search/tests/xpcshell/test_645970.js
@@ +7,5 @@
>  /**
>   * Test nsSearchService with nested jar: uris, without async initialization
>   */
>  function run_test() {
> +  updateAppInfo();  

Nit: there is trailing whitespace on this line.
Comment 5 Joshua Kalpin 2013-02-15 06:28:04 PST
Created attachment 714350 [details] [diff] [review]
Second patch
Comment 6 Joshua Kalpin 2013-02-15 06:29:34 PST
Created attachment 714351 [details] [diff] [review]
Actual second patch

Goofed on the first file upload. This is the correct one.
Comment 7 Ryan VanderMeulen [:RyanVM] 2013-02-15 08:47:46 PST
Comment on attachment 714351 [details] [diff] [review]
Actual second patch

In my landing queue. In the future, please use the checkin-needed keyword at the top instead (we typically use checkin? for bugs with multiple attached patches and only a subset needing checkin).
Comment 8 Ryan VanderMeulen [:RyanVM] 2013-02-15 08:48:36 PST
But thank you for the patch!
Comment 9 Ryan VanderMeulen [:RyanVM] 2013-02-15 08:59:13 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/6f47066fa6fd
Comment 10 Ryan VanderMeulen [:RyanVM] 2013-02-15 13:38:51 PST
https://hg.mozilla.org/mozilla-central/rev/6f47066fa6fd

Note You need to log in before you can comment on or make changes to this bug.