Closed Bug 496727 Opened 12 years ago Closed 12 years ago

Clean up the code landed as part of bug 488706

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 3.6a1

People

(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)

References

Details

Attachments

(1 file, 1 obsolete file)

Follow-up from bug 488706 comment 51.  I quote Mossop's comment there here for easier reference:

> FWIW I think we should avoid the (function() { })() style, I don't see a real
> need for it in this case and it is quite different to the general style in
> browser.js. I'd also question why we didn't use the migration path that already
> exists in nsBrowserGlue.js rather than rolling a new one. I guess the
> BrowserGlue is in the direct startup I don't think it should be causing enough
> of a hit to be concerned about.

He also mentioned on IRC that I should have used gPrefService, which is right.

mconnor, how would you like me to proceed here?  Do you want me to address all three items, or maybe some others as well?
Ehsan, ping?
Component: Preferences → General
QA Contact: preferences → general
Attached patch Patch (v1) (obsolete) — Splinter Review
Don't use the (function() {}() style and use gPrefService.  No other changes in this patch.  I don't think we should use the browser glue migration path because it's used for UI migration and it's absolutely needed before the UI appears, but this one is not, and I hate to add something to the browser startup cost when it's not needed there, as little as it may be.
Attachment #388457 - Flags: review?(dao)
Attachment #388457 - Flags: review?(dao) → review+
Comment on attachment 388457 [details] [diff] [review]
Patch (v1)

Should also use let instead of var here, and Cu instead of Components.utils.
Attached patch For check-inSplinter Review
(In reply to comment #3)
> (From update of attachment 388457 [details] [diff] [review])
> Should also use let instead of var here, and Cu instead of Components.utils.

Done.
Attachment #388457 - Attachment is obsolete: true
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/e042986c85f2
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.6a1
You need to log in before you can comment on or make changes to this bug.