573726 - Simplify Utils.js

Status: RESOLVED INVALID

(Firefox for Android Graveyard :: General, defect)

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

Utils.js is loaded into any tabs but there are many parts of it that are not really needed on the content side.

Comment 1

[Matt Brubeck (:mbrubeck)]

I like most of this, but:
- getHrefForElement should stay on the content side.
- I'd like to keep the Util object in Util.js or change the filename, otherwise I will get confused when looking for it.  The real "utility" functions like bind and executeSoon will probably be wanted just as often in content as in chrome, and should stay in Util.js.  (The same is probably true of dumpf and dumpln.)

If we do keep Util in Util.js (or maybe even if we don't), then:
- isPortrait can move to BrowserUI.
- makeURLAbsolute is only used in BrowserUI._getLinkURL.  It could be moved to BrowserUI or just inlined there.
- needHomepageOverride can move to Browser.

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #1)
> I like most of this, but:
> - getHrefForElement should stay on the content side.
> - I'd like to keep the Util object in Util.js or change the filename, otherwise

I would like to have a filename called geometry.js

> I will get confused when looking for it.  The real "utility" functions like
> bind and executeSoon will probably be wanted just as often in content as in
> chrome, and should stay in Util.js.  (The same is probably true of dumpf and
> dumpln.)

First i agree that it is confusing and we should change the name, also personnaly I will be happy to get rid as much as possible of the "bind" call we used to have and I see dumpf and dumpln as debug utilities functions that doesn't really need to live somewhere in the code. I would rather prefer have a file called debug.js and loaded everywhere while in debug build.

What do you think?


> If we do keep Util in Util.js (or maybe even if we don't), then:
> - isPortrait can move to BrowserUI.
> - makeURLAbsolute is only used in BrowserUI._getLinkURL.  It could be moved to
> BrowserUI or just inlined there.
> - needHomepageOverride can move to Browser.

Agreed with that. I 'll update the patch.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Also remember the reflow problem with using content.scrollX and scrollY instead of using Util.getScrollOffset()

Comment 4

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Now that we have a Geometry.jsm and other stuffs I don't think this make sense