Remove redundant NewTabUtils.jsm import from browser-thumbnails.js

RESOLVED FIXED in Firefox 38

Status

()

Firefox
New Tab Page
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: dao, Assigned: curtisk, Mentored)

Tracking

Trunk
Firefox 38
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
+++ This bug was initially created as a clone of Bug #1125497 +++

At the top of browser/base/content/browser-thumbnails.js, we import NewTabUtils.jsm because some code in browser-thumbnails.js wants to use NewTabUtils. We can remove this import, because browser.js already imports NewTabUtils.jsm (http://hg.mozilla.org/mozilla-central/annotate/38e4719e71af/browser/base/content/browser.js#l29).
(Assignee)

Comment 1

3 years ago
I've always wanted to land a bug and learn how this works and this seems like a good one to learn on. :dao are you the mentor?

Comment 2

3 years ago
Created attachment 8555512 [details] [diff] [review]
Bug-1126253.patch
Attachment #8555512 - Flags: review?(dao)
(Reporter)

Comment 3

3 years ago
(In reply to Curtis Koenig [:curtisk (non-Moz)] from comment #1)
> I've always wanted to land a bug and learn how this works and this seems
> like a good one to learn on. :dao are you the mentor?

I am...
(Reporter)

Comment 4

3 years ago
Comment on attachment 8555512 [details] [diff] [review]
Bug-1126253.patch

>--- a/browser/base/content/browser.js
>+++ b/browser/base/content/browser.js
>@@ -21,18 +21,16 @@ XPCOMUtils.defineLazyModuleGetter(this, 
> XPCOMUtils.defineLazyModuleGetter(this, "Task",
>                                   "resource://gre/modules/Task.jsm");
> XPCOMUtils.defineLazyModuleGetter(this, "CharsetMenu",
>                                   "resource://gre/modules/CharsetMenu.jsm");
> XPCOMUtils.defineLazyModuleGetter(this, "ShortcutUtils",
>                                   "resource://gre/modules/ShortcutUtils.jsm");
> XPCOMUtils.defineLazyModuleGetter(this, "GMPInstallManager",
>                                   "resource://gre/modules/GMPInstallManager.jsm");
>-XPCOMUtils.defineLazyModuleGetter(this, "NewTabUtils",
>-                                  "resource://gre/modules/NewTabUtils.jsm");

Sorry, that's the wrong way around. We need to remove the import from browser-thumbnails.js, not from browser.js.
Attachment #8555512 - Flags: review?(dao) → review-
(Assignee)

Comment 5

3 years ago
so I think I have this figured out (what to change for the appropriate patch) but I am confused on MozReview on how to submit this patch and I can't seem to find you (:dao) on irc
(Assignee)

Comment 6

3 years ago
Created attachment 8556241 [details] [diff] [review]
bug_1126253.patch
Attachment #8556241 - Flags: review?(dao)
(Reporter)

Comment 7

3 years ago
Comment on attachment 8556241 [details] [diff] [review]
bug_1126253.patch

looks good!
Attachment #8556241 - Flags: review?(dao) → review+
(Reporter)

Comment 8

3 years ago
https://hg.mozilla.org/integration/fx-team/rev/a8feff978f82
Assignee: nobody → curtis.koenig+bz
(Assignee)

Updated

3 years ago
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/a8feff978f82
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 38
You need to log in before you can comment on or make changes to this bug.