833207 - Social callbacks are made when panel/chat loads the error page.

Status: RESOLVED WONTFIX

(Firefox Graveyard :: SocialAPI, defect)

Description

[Mark Hammond [:markh] [:mhammond]]

If a provider attempts to create a panel or chat window, and the panel load fails causing about:socialerror to be displayed, the provider-specified callback is still made.  This callback will then fail as the provider will be expecting their content rather than about:socialerror.

Comment 1

[Shane Caraveo (:mixedpuppy)]

I thought about returning null if provider.errorState was set.  The problem is that would leave the flyout "orphaned", though I'm not sure that is really a problem.  Essentially, the provider would not be able to close the flyout, or the chat window, programmatically.

Comment 2

[Mark Hammond [:markh] [:mhammond]]

(In reply to Shane Caraveo (:mixedpuppy) from comment #1)
> I thought about returning null if provider.errorState was set.  The problem
> is that would leave the flyout "orphaned", though I'm not sure that is
> really a problem.  Essentially, the provider would not be able to close the
> flyout, or the chat window, programmatically.

I believe that the flyout is "orphaned" regardless - even if we decided to keep making the callback, the fact we fail to inject any mozSocial stuff, including the trick to allow window.close(), means there is nothing they could do with the window anyway.

Comment 3

[Shane Caraveo (:mixedpuppy)]

(In reply to Mark Hammond (:markh) from comment #2)
> (In reply to Shane Caraveo (:mixedpuppy) from comment #1)
> > I thought about returning null if provider.errorState was set.  The problem
> > is that would leave the flyout "orphaned", though I'm not sure that is
> > really a problem.  Essentially, the provider would not be able to close the
> > flyout, or the chat window, programmatically.
> 
> I believe that the flyout is "orphaned" regardless - even if we decided to
> keep making the callback, the fact we fail to inject any mozSocial stuff,
> including the trick to allow window.close(), means there is nothing they
> could do with the window anyway.

Then we need to either not make the callback, or send null as the param on the callback.

Comment 4

[Mark Hammond [:markh] [:mhammond]]

(In reply to Shane Caraveo (:mixedpuppy) from comment #3)
> (In reply to Mark Hammond (:markh) from comment #2)
> > (In reply to Shane Caraveo (:mixedpuppy) from comment #1)
> > > I thought about returning null if provider.errorState was set.  The problem
> > > is that would leave the flyout "orphaned", though I'm not sure that is
> > > really a problem.  Essentially, the provider would not be able to close the
> > > flyout, or the chat window, programmatically.
> > 
> > I believe that the flyout is "orphaned" regardless - even if we decided to
> > keep making the callback, the fact we fail to inject any mozSocial stuff,
> > including the trick to allow window.close(), means there is nothing they
> > could do with the window anyway.
> 
> Then we need to either not make the callback, or send null as the param on
> the callback.

Right - I thought you were saying that sending null or not making the callback had the downside of leaving the flyout orphaned.  I was pointing out that it is orphaned regardless.

IMO, we should not make the callback.  This leaves us open in the future to automatically "fixing" the error (eg, detecting the network came up), reloading the originally requested URL and making the callback at that point.

Comment 5

[Shane Caraveo (:mixedpuppy)]

(In reply to Mark Hammond (:markh) from comment #4)

> Right - I thought you were saying that sending null or not making the
> callback had the downside of leaving the flyout orphaned.  I was pointing
> out that it is orphaned regardless.

I was, I didn't really think about it much since I was working on the main patch.

> IMO, we should not make the callback.  This leaves us open in the future to
> automatically "fixing" the error (eg, detecting the network came up),
> reloading the originally requested URL and making the callback at that point.

Passing null back would let the provider know there was an error, that's the only reason I was thinking it might be useful.

Comment 6

[Shane Caraveo (:mixedpuppy)]

deprecation in fx51