1346188 - Remove redundant AppConstants.jsm import from browser.js

Status: RESOLVED FIXED

(Firefox :: General, enhancement)

Description

[Dão Gottwald [:dao]]

The lazy AppConstants.jsm import in browser.js isn't needed since utilityOverlay.js already imports AppConstants.jsm:

http://searchfox.org/mozilla-central/rev/7cb75d87753de9103253e34bc85592e26378f506/browser/base/content/browser.js#41

http://searchfox.org/mozilla-central/rev/7cb75d87753de9103253e34bc85592e26378f506/browser/base/content/utilityOverlay.js#7

Comment 1

[Wickie Lee [:Robocrypt]]

I would be interested in working on this. Mainly just to get familiar with the processes involved. Though I have no idea where to start or how to create and submit a patch for this.

Comment 2

[Dão Gottwald [:dao]]

(In reply to Wickie Lee [:Robocrypt] from comment #1)
> I would be interested in working on this. Mainly just to get familiar with
> the processes involved. Though I have no idea where to start or how to
> create and submit a patch for this.

Have you built Firefox yet? See https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction

Comment 3

[Wickie Lee [:Robocrypt]]

(In reply to Dão Gottwald [::dao] from comment #2)
> (In reply to Wickie Lee [:Robocrypt] from comment #1)
> > I would be interested in working on this. Mainly just to get familiar with
> > the processes involved. Though I have no idea where to start or how to
> > create and submit a patch for this.
> 
> Have you built Firefox yet? See
> https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction

Yep, I have already built Firefox. I guess my confusion comes from the process of creating patch, the review process (MozReview vs Bugzilla), such as how to perform either or which one do I use. And then actually getting the patch into Firefox.

Comment 4

[Dão Gottwald [:dao]]

Either bugzilla or mozreview are fine. You can get patches landed using the checkin-needed keywords in bugzilla, but let's not worry about that for now.

Comment 5

[Federico Padua [:fedepad]]

Since it's been 6 days since last update and this bug has not been assigned I'm pushing something for review.

Thanks.

Comment 6

[Federico Padua [:fedepad]]

Attachment: Bug 1346188 - Remove redundant AppConstants.jsm import from browser.js;

This patch removes the redundant lazy import of AppConstants.jsm from
browser/base/content/browser.js since it gets loaded previously as described
in the bug description.

Review commit: https://reviewboard.mozilla.org/r/121762/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/121762/

Comment 7

[Dão Gottwald [:dao]]

Comment on attachment 8848887 [details]
Bug 1346188 - Remove redundant AppConstants.jsm import from browser.js;

https://reviewboard.mozilla.org/r/121762/#review124074

Thanks!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a6463366885d
Remove redundant AppConstants.jsm import from browser.js; r=dao

Comment 9

[Federico Padua [:fedepad]]

Comment on attachment 8848887 [details]
Bug 1346188 - Remove redundant AppConstants.jsm import from browser.js;

https://reviewboard.mozilla.org/r/121762/#review124074

You're welcome! Thank to you!

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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