84910 - Split up navigator.js

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Doron Rosenberg (IBM)]

As per jag/ben, since viewsource now needs a few functions from navigator.js, we
should split that file into a shared file (calling it common.js for the moment)
and keep navigator.js for functions only needed by navigator.  Still not sure
about hiddenWindow.xul though and what it really needs.

Currently in my common.js is:

getBrowser()
BrowserOpenWindow()
BrowserPrintPreview() which is empty still
BrowserPrint()
BrowserSetDefaultCharacterSet() 
BrowserSetForcedCharacterSet()
BrowserSetForcedDetector()
BrowserClose()
BrowserFInd()
BrowserFIndNext()
BrowserCanFindNext()

I'll modify this if the findprevious patch lands before, as it renames the
BrowserFind() functions.

Any comments? I'll file a navigatorOverlay.xul bug too

Comment 1

[timeless]

how about calling it browser.js ?

Comment 2

[Doron Rosenberg (IBM)]

Attachment: patch, needs some refinement

Comment 3

[Doron Rosenberg (IBM)]

created a working patch, unless someone again changes navigator.js :)

browser.js now has all functions shared between the browser and viewsource. I
tested this and seems that everything is fine. 

Comment 4

[Doron Rosenberg (IBM)]

ok, this does not break anything, navigator and viewsource work fine. Any
comments? or even review?

Comment 5

[jag (Peter Annema)]

Comment on attachment 52455 [details] [diff] [review]
patch, needs some refinement

r=jag

Comment 6

[Doron Rosenberg (IBM)]

i just realised that it would be more logical to keep in navigator.js the
functions we need in both and browser.js to have the functions only the browser
needs. New patch coming

Comment 7

[Doron Rosenberg (IBM)]

Attachment: better patch - make navigator.js the global file and browser.js the browser-specific file

Comment 8

[Doron Rosenberg (IBM)]

new patch, pretty big cause it moves most of navigator.js to browser.js.
browser.js is now only for the browser, while navigator.js is for the shared foo
between viewsource and navigator.

Argh, noticed I forgot to diff hiddenWindow.xul. Any comments before I add a new
one?

Comment 9

[jag (Peter Annema)]

Yeah.

Your old way was better, I think.

<browser>     -> browser.js
navigator.xul -> navigator.js

Comment 10

[Doron Rosenberg (IBM)]

Attachment: new patch based on the first one, fixes character coding

Comment 11

[Doron Rosenberg (IBM)]

attached patch fixes character coding in viewsource (it needed |const
nsIWebNavigation = Components.interfaces.nsIWebNavigation;|).

based on the first patch, as jag correctly points out it makes more sense. r/sr?

Comment 12

[jag (Peter Annema)]

Comment on attachment 53593 [details] [diff] [review]
new patch based on the first one, fixes character coding

I would keep getBrowser() in navigator.js (and the copy in viewsource.js), since
it's tied to the id that's put on the <browser> (or <tabbrowser> as it may so be).
I don't think you necessarily want to force viewsource to use the same id on its
<browser> as navigator. Make sure your |var gBrowser| is declared everywhere you
need it.

Also, BrowserClose() really belongs in navigator.js, I think.

Other than that it looks good.

Comment 13

[Doron Rosenberg (IBM)]

Attachment: new patch with jag's changes

Comment 14

[Doron Rosenberg (IBM)]

patch with jag's changes, added a ViewSourceClose() function to viewsource.

Comment 15

[Doron Rosenberg (IBM)]

pushing to 0.9.7 due to no review

Comment 16

[jag (Peter Annema)]

Comment on attachment 54408 [details] [diff] [review]
new patch with jag's changes

r=jag (sorry, I thought I had already added this)

Comment 17

[Doron Rosenberg (IBM)]

Attachment: new patch due to some changed to navigator.js

Hyatt added BrowserCloseTabOrWindow(), and 2 print functions got changed
(PrintPreview() and PrintSetup()), new patch reflects those changed.

Comment 18

[Doron Rosenberg (IBM)]

Comment on attachment 58101 [details] [diff] [review]
new patch due to some changed to navigator.js

has a jar.mn change to include a file that is part of another patch

Comment 19

[Doron Rosenberg (IBM)]

Attachment: new patch

new patch is without the jar.mn line which was for another patch

Comment 20

[Doron Rosenberg (IBM)]

ok, since I've got 110506 working (split navigatorOverlay), and since this
blocks that bug, could I get review for this?

thanks!

Comment 21

[jag (Peter Annema)]

Doron, I just made some more changes to navigator.js. Could you update your
patch? With those changes, r=jag.

Comment 22

[Doron Rosenberg (IBM)]

Attachment: applies now after jag's changes

new patch, reflects changes to navigator.js

Comment 23

[jag (Peter Annema)]

Comment on attachment 60335 [details] [diff] [review]
applies now after jag's changes

r=jag

Comment 24

[Doron Rosenberg (IBM)]

cc: alecf for possible comments/sr

Comment 25

[Alec Flett]

Comment on attachment 60335 [details] [diff] [review]
applies now after jag's changes

doron, you rock! Assuming this is just moving code around & updating .jar
files, sr=alecf

Comment 26

[Doron Rosenberg (IBM)]

checked in by hwaara

Comment 27

[sairuh (rarely reading bugmail)]

rs vrfy.