Closed Bug 758812 Opened 10 years ago Closed 9 years ago

Break up browser.js into smaller pieces, Phase 1

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 15

People

(Reporter: Dolske, Assigned: Dolske)

References

Details

(Keywords: dev-doc-needed)

Attachments

(4 files, 1 obsolete file)

It is widely acknowledged that browser.js is too damn bug. The in-tree version is currently 9431 lines long. (It's actually 13,193 lines if you count the 6 files that are already #included)

The first step to fixing our problem is to blast out some of the big and obvious chunks into separate files and #include them. If nothing else, this starts to make looking at code history easier (because history for browser-foo.js will be separate from browser-bar.js).

My plan is to start with splitting out things like "var gFoo = { ... }" in a few steps. Later on, when the dust settles, we can look at moving related global functions into their own files as well.
(I'm only going to set the review flag on this first patch, because multiple flags are so annoying. The intention is for all to be reviewed. Note that there are no code changes here -- just moving chunks and adding #includes)
Assignee: nobody → dolske
Attachment #627395 - Flags: review?(gavin.sharp)
Attached patch Patch for FeedHandler (obsolete) — Splinter Review
Comment on attachment 627397 [details] [diff] [review]
Patch for FeedHandler

Oops, I seem to have merged the FeedHandler and FullScreen changes. Meh.
Attachment #627397 - Attachment is obsolete: true
I did a try run just to make sure nothing unexpected broke. It's ok.

https://tbpl.mozilla.org/?tree=Try&rev=532c60405f19

It also occurred to me that browser-lwt.js could be grouped into browser-addons.js.
Attachment #627395 - Flags: review?(gavin.sharp) → review+
Attachment #627398 - Attachment description: Patch for FullScreen → Patch for FullScreen and feeds
Attachment #627398 - Flags: review+
Duplicate of this bug: 381210
Any reason why LightWeightThemeInstaller isn't in browser-addons.js?
I don't feel strongly; it seems pretty self-contained and large enough to merit its own file.
Well, gXPInstallObserver and AddonsMgrListener are similarly self-contained.
(In reply to Dão Gottwald [:dao] from comment #9)
> Any reason why LightWeightThemeInstaller isn't in browser-addons.js?

From comment 7:

> It also occurred to me that browser-lwt.js could be grouped into browser-addons.js.

I was just going to do that for checkin.
Summary: Break up browser.js into smaller pieces (Part 1) → Break up browser.js into smaller pieces, Phase 1
This will need to be mentioned to devs that poke around within the product; most already know, but it's still worth noting on Firefox 15 for developers.
Keywords: dev-doc-needed
Sheppy: doesn't hurt, but I'd be careful to note that there's essentially no change to the browser.js that _ships_ with Firefox, it's only the files as they live in mozilla-central that have been split. (The magic of #include.)
You need to log in before you can comment on or make changes to this bug.