68086 - Generic about: redirector

Status: VERIFIED FIXED

(Core :: Networking, enhancement)

Description

[Ben Bucksch (:BenB)]

We have at least 3 classes which just redirect, more likely to come. This is
code redundancy. We should have one class which keeps a list of URLs to redirect.

Note that this class must not assume "default" ownership of about:* - there
might be other about URL types similar to this one, which don't have hardcoded
about: URLs. (E.g. an about:<developer>, like in 4.x., which doesn't use static
URLs, but somehow figures the developer names and their URL out from the
Contributors list or some other magic).

Comment 1

[Ben Bucksch (:BenB)]

Any ideas how to implement it? I couldn't figuer it our back in 1999.

Comment 2

[Brendan Eich [:brendan]]

Shouldn't be too hard, eh?  Cc'ing netwerk gurus.

/be

Comment 3

[Darin Fisher]

shouldn't be too hard, but it's definitely well placed in the "future
enhancement" category... we have much more pressing issues at hand ;-)

Comment 4

[Gagan]

->moi

Comment 5

[Gagan]

can't do future-- we need to clean this up sooner.

Comment 6

[Ben Bucksch (:BenB)]

Thanks gagan. I'm willing to assist you.

Comment 7

[Gagan]

Attachment: Diffs from mozilla/netwerk

Comment 8

[Gagan]

The two new files nsAboutRedirector.[h|cpp] have been checked in. Pls review 
those as well. 

http://lxr.mozilla.org/seamonkey/source/netwerk/protocol/about/src/nsAboutRedire
ctor.cpp
and 
http://lxr.mozilla.org/seamonkey/source/netwerk/protocol/about/src/nsAboutRedire
ctor.h

Comment 9

[rpotts (gone)]

Hi Gagan,

these changes look good to me :-)  i'll give you an r/sr=rpotts (whichever).  
lets check it in and clean some stuff up !

- -rick

Comment 10

[Darin Fisher]

r/sr=darin

Comment 11

[Ben Bucksch (:BenB)]

Gagan, thanks for fixing this.

darin, rpotts, you are too fast ;-P.

FWIW, the changes look good to me "on paper", modulo comments below. If I only
knew it would be that easy, I'd implemented it myself. :-) Will test the change
when the update / compile finishes (probably tomorrow - 4AM here).

Comments:
- I assume you will change the makefiles for all platforms before you check in.
- I would have written

static const int kRedirTotal = 3;
static const char *kRedirMap[kRedirTotal][2] = {
...

Comment 12

[Ben Bucksch (:BenB)]

> - I would have written

(of course, that's only style.)

Thanks again.

Comment 13

[Gagan]

fix is in. 

Comment 14

[Ben Bucksch (:BenB)]

The files nsAboutCredits.*, nsAboutPlugins.* and mzAboutMozilla.* are still in
the tree (and active). Should I remove them? (r=? sr=?)

tever, should I do the verification?

Comment 15

[Ben Bucksch (:BenB)]

(active = not cvs removed. They are not part of the build anymore, as far as I
can see.)

Comment 16

[Brendan Eich [:brendan]]

Clear removals thru leaf.

/be

Comment 17

[Gagan]

yup I left them to make sure the transition went smooth. Now that all platforms 
are running smoothly we should be able to remove them. Leaf should still clear 
this.

Comment 18

[Daniel (Leaf) Nunes]

It would be ideal to have someone with a mac build remove the files in question 
from their tree and rebuild, since i can't check mac projects for references to 
them. Any mac buddies care to remove the three files before their next build?

Comment 19

[benc]

need an easy set of steps to verify this...

Comment 20

[Bradley Baetz (:bbaetz)]

This works, because I stepped through the code about a month ago.

VERIFIED.

Comment 21

[Alexey Chernyak]

all old files have been removed in bug 123580

about:config needs to be done as well, see bug 127252

Comment 22

[Alexey Chernyak]

can different security levels for different URLs be implemented for redirector?
see bug 127252

Comment 23

[Darin Fisher]

cc'ing mstoltz... see comment #22