Break up browser.js into smaller pieces, Phase 1

RESOLVED FIXED in Firefox 15

Status

()

Firefox
General
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Dolske, Assigned: Dolske)

Tracking

({dev-doc-needed})

unspecified
Firefox 15
dev-doc-needed
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
Created attachment 627395 [details] [diff] [review]
Patch for gPluginHandler

(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)
(Assignee)

Comment 2

5 years ago
Created attachment 627396 [details] [diff] [review]
Patch for LightWeightThemeInstaller
(Assignee)

Comment 3

5 years ago
Created attachment 627397 [details] [diff] [review]
Patch for FeedHandler
(Assignee)

Comment 4

5 years ago
Created attachment 627398 [details] [diff] [review]
Patch for FullScreen and feeds
(Assignee)

Comment 5

5 years ago
Created attachment 627399 [details] [diff] [review]
Patch for 2 addon things
(Assignee)

Comment 6

5 years ago
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
(Assignee)

Comment 7

5 years ago
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 #627396 - Flags: review+
Attachment #627398 - Attachment description: Patch for FullScreen → Patch for FullScreen and feeds
Attachment #627398 - Flags: review+
Attachment #627399 - 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.
(Assignee)

Comment 12

5 years ago
(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.
(Assignee)

Updated

5 years ago
Summary: Break up browser.js into smaller pieces (Part 1) → Break up browser.js into smaller pieces, Phase 1
(Assignee)

Comment 13

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/2ce67e30d650
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc6ca4cba764
https://hg.mozilla.org/integration/mozilla-inbound/rev/9bb221edc54e
https://hg.mozilla.org/integration/mozilla-inbound/rev/729a6b7922cd
https://hg.mozilla.org/mozilla-central/rev/729a6b7922cd
https://hg.mozilla.org/mozilla-central/rev/9bb221edc54e
https://hg.mozilla.org/mozilla-central/rev/cc6ca4cba764
https://hg.mozilla.org/mozilla-central/rev/2ce67e30d650
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 15
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
(Assignee)

Comment 16

5 years ago
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.)
See Also: → bug 872761
You need to log in before you can comment on or make changes to this bug.