27048 - need http clients to implement nsIHTTPEventSink

Status: VERIFIED FIXED

(Core :: Networking, defect, P3)

Description

[Gagan]

so that we can move auth-prompting, redirects updates to the client.

Comment 1

[Warren Harris]

Who are the clients besides webshell? Should this be assigned to Travis?

Comment 2

[Gagan]

I think its just the webshell for now... off to travis. Once this is done it 
will also help us crack other bugs relating to URI updates on redirects (see bug 
 20072, bug 10647)

Comment 3

[Gagan]

taking over... (and marking PDT+ thru property of transitive associativeness) :)

Comment 4

[Gagan]

the webshell has changed significantly last night and is going to continue to
evolve (per travis) and since the changes rely on the structure of the webshell,
this has an indeterminate ETA.

Comment 5

[Gagan]

*** Bug 9472 has been marked as a duplicate of this bug. ***

Comment 6

[Gagan]

*** Bug 20072 has been marked as a duplicate of this bug. ***

Comment 7

[Gagan]

updating eta

Comment 8

[Gagan]

There are two pieces of this problem-- one that necko needed to call events on
the eventsink (the part that I have done and is ready for review/checkin) and a
second part that makes the webshell (or as per travis the uriloader) implement
the interface to do the actual thing-- ranging from throwing the auth dialog box
to updating the location bar based on the events from HTTP. Jar recommends that
we check in both the pieces at reasonably close enough intervals otherwise we
would have a period of severe regression. And so I am going to hold off my
changes till I can get an implementor for nsIHTTPEventSink and nsIPrompt
(travis/mscott)

Comment 9

[Jim Roskind]

Gagan: Can you put a dependency in for the bug that Travis/Mscott would be
working on? We can make the two mutually dependendent... but we need to be sure
that both are on the beta1 PDT+ radar.

Thanks

Comment 10

[Gagan]

this bug itself was really intended for that purpose... however I landed up
changing part of HTTP event sink in doing so. So if you want I can create a
separate bug or just leave this as the main one.  

Comment 11

[kevinyen]

Adding myself to cc and adding beta1 per beta bug #9472.
-kevin

Comment 12

[Scott MacGregor]

Gagan, I can help out and take a look at implementing this in the doc loader for
now. I've got another PDT bug I'm working on right now and when I'm done i'll
start looking at this one.

Comment 13

[Scott MacGregor]

Hey gagan, if i add http event sink support to the doc loader, what exactly do
you expect the doc loader to do when it's told things like:
 OnhttpHeadersAvailable, etc.

Comment 14

[kevinyen]

Adding [Activation] to summary field, since Activation depends on this 

(0rig Activation bug was 28730, which was a dup of 9472, which was a dup of 
27048, which is a dup of this bug.)

thx,
kevin

Comment 15

[kevinyen]

I know everyone is cranking away very hard, but may I ask what the anticipated 
date of the parallel landing is?

Turning Activation on and getting Activation to work bugfree is dependent on 
this.

thx,
kevin

Comment 16

[racham]

*** Bug 30287 has been marked as a duplicate of this bug. ***

Comment 17

[Scott MacGregor]

Kevin: I don't know. I haven't even started to look at this bug to see what work
Gagan needs help with for the doc loader. On a side note, can you explain what
activation has to do with implmenting a http event sink in the doc loader? I'm
missing the big picture and am just curious. (also, what is activation?)

Comment 18

[kevinyen]

1.  What is Activation?  Briefly, Activation is :
      - NC registration integrated with the install process.
      - Necessary to activate server-based features, such as AIM, Webmail, 
Calendar, Radio, etc.
      - Dialog-like pages served from Netcenter into a chromeless browser 
window

2. How does Activation relate to this bug?  Basically, 
    - 0rig Activation bug was 28730, which was a dup of 9472, which was a dup of 
27048
    - 28730 says that a js function(GetQueryParameter) result is not being 
received.  Bhuvan and I checkout out the js and it looks fine.

Also, marking Confidential per all Activation bugs, and it's ok because all 
folks in cc are netscape.com

thx,
kevin

Comment 19

[travis]

I still don't quite get it.  What feature does nsIHTTPEventSink being 
implemented give us?

Comment 20

[Scott MacGregor]

Kevin, I'm trying to go back through the activation bugs that eventually led to
involve this bug (I don't understand how this applies to activation yet).

You listed 28730 as the original activation bug but that's a mailnews
bug. Maybe you have the wrong number?

Comment 21

[kevinyen]

Here's a summary of the Activation bug trail: 28730 --> 9472 --> this bug.

28730: 
- http://ins-group1.netscape.com:9999/activation/en/activation.cgi
- To replicate original bug:
  1. sign into netcenter (sets nc cookie on your computer)
  2. load the above url
  3. The first radio buttons says "My Netscape user name is."  The bug is that 
your user name is missing at the end of the sentence.  It works fine in M13.

- This js found at the above URL works in M13 but not in recent builds:
  userid = GetQueryParameter(query, "userid")

- The problem is that we seem to lose the search parameters when we do a 302 
redirection (hence window.location.search returns nothing). A DUP of bug 9472.

9472:
- window.location.search is not set when a HTTP 302 redirect occurs to a page
with a search string which is how Messenger Express server passes arguments to
the client application

- The original URL specified is not valid. I did find a place on Netcenter where 
a HTTP 302 redirect takes you to a page with a search component (a ? in the 
URL). It seems like netlib is stripping out or somehow losing the search 
component. As a result, querying it through JavaScript returns the wrong answer.

- yes this is a definite dup of bug 27048 and norris we need to merge in our 
changes...


27048 is this bug.

I hope this helps instead of confuses.

thx, kevin

Comment 22

[kevinyen]

mscott, 
I believe 28730 is the right bug to begin with.  I don't get a mail/news when I 
load: http://bugzilla.mozilla.org/show_bug.cgi?id=28730

thx,
kevin

Comment 23

[Gagan]

mscott: Essentially nsIHTTPEventSink will provide two functions- one for
OnHeadersAvailable (which for now would be a no-op but would essentially allow
anyone to do a GetContentType and maybe retargetting if required) the other
function is OnRedirect which would give the correct URL to which we are
redirecting. This should be used to update the location bar. (which is why bugs
like 9472 become dups)

Comment 24

[Jim Roskind]

Travis/Mscott: When will the other half of this be ready?  Please put this 
landing date into the status whiteboard, or include a reference to the bug for 
the other "half" in the "depends on" list above.
Thanks,
Jim

Comment 25

[Scott MacGregor]

Jar, travis and I don't have another bug to track our part for this. I was going
to use this one. Let me know if you'd rather have us file a separate one. As far
as a timeline goes. We just started looking at the work gagan needs us to do on
Friday. We need to talk to gagan to get a better understanding of what (and
where) our stuff needs to do.

We'll post an estimate after we talk to Gagan on monday.

Comment 26

[Paul MacQuiddy]

[e-mail] would like to look at this bug, is there a good 
reason this is netscape confidential?

Comment 27

[kevinyen]

Removing confidential marking.

-kevin

Comment 28

[Scott MacGregor]

Current Status: Gagan, Travis and I spent some time looking at this this
afternoon and we think we don't need to fix this bug for beta. Norris checked in
a bug fix Thursday or Friday which fixed the redirection problem. We checked the
bugs that were depending (or marked as dups of this bug) involving redirecting
urls and they were all working now.

We still need to implement nsIHTTPEventSink. Just not for beta.

Activation group: you guys should be unblocked from your dependency on this bug.

Clearing the pdt+ resolution so the pdt team can re-evaluate.

Comment 29

[Scott MacGregor]

I would also add that the only problem we saw with redirection was the fact that
the url bar isn't being updated with the new redirected url. But relative links
inside of the new redirected document were all resolved correctly.

Comment 30

[leger]

Putting on PDT+ for beta1. Please fix by 03/08

Comment 31

[leger]

Putting on PDT- for beta1.   Please open new bug on location bar issue.

Comment 32

[Scott MacGregor]

Here's the bug I filed for updating the location bar on redirects: 30783

Comment 33

[Gagan]

FWIW I have checked in the HTTP side of the story. I have also checked in
changes on the webshell/docshell to return a valid nsIPrompt from the
GetInterface calls. So things should work fine now. 

Comment 34

[Jim Roskind]

IF this is completely fixed (as per comment from gagan), this really needs to be
marked fixed, and the dependent PDT+ 27797 bug needs to be closed (marked fixed).
Thanks,
Jim

Comment 35

[kevinyen]

I don't think it's fixed completely.

Grace -- Assuming the NC activation service is up, we'll see this bug remains:

  - On the Confirmation screen (Activation Complete), the user name is still 
missing.

The first half of the problem seems to have been fixes -- on cookie-exists 
Invitation screen, the lookup does occur.

Will provide more details as soon as I can, but want to type this quick note 
now.

thx,
kevin

Comment 36

[Jim Roskind]

Hmm... if this is not fixed... then it is holding up a PDT+ bug 27797 (to some
extent).

What is the plan/story?  Do we have to release note the other bug??

Thanks,
Jim

Comment 37

[racham]

It is realized that the actual problems were in javascript sources on activation 
servers, as things started working fine once activation team fixed them. Once 
they reached the place where they called window.location.search, the client is 
doing the right thing (in the activation case). I have already removed bug 
27797's dependency on this one.

Comment 38

[Warren Harris]

Moving to M15

Comment 39

[Gagan]

this should now be owned by someone in webshell... to travis

Comment 40

[kmurray1115]

Travis is gone. Who's looking at this?

Comment 41

[selmer (gone)]

Scott, are you the de-facto owner now?

Comment 42

[leger]

Need to know if this is still a problem to get activation up and running?

Comment 43

[racham]

I don't think activation is depending on this anymore. We had activation up and 
running in PR1 (and unitl couple of builds which now suffers with some docshell 
problem) and I think the parts required for activation are fixed.

Gagan or Scott should at look why this bug is origianlly opened and if there are 
any other issues that need to be addressed/fixed with this bug.

Comment 44

[leger]

Putting on [nsbeta2-] radar.  

Comment 45

[Andreas Franke (gone)]

How can this bug be [nsbeta2-] when it's blocking several [nsbeta2+] bugs?
Here's the current status of some of the dependent bugs:
bug 39439 [nsbeta2+]
bug 30271 [nsbeta2+]
bug 28594 [nsbeta2+] 6/1
bug 28459 [nsbeta2+] 6/1
bug 25684 [nsbeta2+] (crasher)
Removing [nsbeta2-] from Status Whiteboard for reconsideration, cc self.

Comment 46

[leger]

Putting on [nsbeta2+] radar for beta2 fix base on blocking issues.  If there are 
workarounds, please remove [nsbeta2+] or re-eval.

Comment 47

[Dan M]

Good catch Andreas: this bug is (has been) considered a major chokepoint for a 
lot of nsbeta2+ bugs, and it was silly to mark it -. However, I should mention 
that we're currently re-evaluating our assumptions that those bugs (modal window 
parenting issues) are actually dependent on this bug. So don't panic yet. A lot 
of dependencies on this bug may be cleared out. More news soon.

Comment 48

[Dan M]

  I believe the modal dialog parenting bugs marked as dependent on this one 
aren't actually. I'd been told they were, but it's looking unrelated to me now 
that I actually look into it. Without being completely certain, I'm clearing 
those dependencies. That leaves this bug as blocking one nsbeta2+ bug: 30271, so 
I can't clear the +.

Comment 49

[Grace Bush]

removing activation from status summary since there no longer is a dependency

Comment 50

[Mike]

M16 has been out for a while now, these bugs target milestones need to be 
updated.

Comment 51

[Scott MacGregor]

I would not pull beta2 off the wire for this. Per the leads meeting today that's
the new litmus test. Adding the nsbeta2-.  You'll see email from the some one up
above real soon now describing these decisions.

I don't believe the bug that depends on this is one I'd pull off the wire for
either.

Comment 52

[Scott MacGregor]

Attachment: proposed fix

Comment 53

[Scott MacGregor]

I attached the changes necessary for proper implementation of nsIHTTPEventSink.
However, the various bugs that depend on this implementation of course require
docshell to do certain things when it is told there is a redirect.....

Comment 54

[Gagan]

note that 30783 got reopened recently and fixing this bug would help fix that 
more conveniently.

Comment 55

[Scott MacGregor]

nsbeta3+ per triage meeeting.

Comment 56

[Scott MacGregor]

the doc loader now implments this interface and calls into the docshell with a 
on location change notification in the case of redirects.

Any future bugs which require docshell to handle this new information will be 
dealt with in other bugs. 

Comment 57

[Tom Everingham]

development issues - verifying