779680 - about:neterror's GoOnline() should be invoked from events, not nsDOMWindowUtils

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Andrew McCreight [:mccr8]]

The current method is gross.

Comment 1

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1

Made the event listeners in browser.js use capture to ensure they get the event first, before the handler in the page. And AFAICT it didn't even break anything!

(Tagging dolske for r? as punishment.)

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 648605 [details] [diff] [review]
Patch v1

r=me

Comment 3

[Justin Dolske [:Dolske]]

Comment on attachment 648605 [details] [diff] [review]
Patch v1

Why the capturing listener? So we trigger online-mode before the page starts the reload?

Probably worth a comment in retryThis that the browser.js code handles the go-online thing.

Comment 4

[Andrew McCreight [:mccr8]]

Is nsDOMWindowUtils::GoOnline still called anywhere after this patch?

Comment 5

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to Andrew McCreight [:mccr8] from comment #4)
> Is nsDOMWindowUtils::GoOnline still called anywhere after this patch?

Bah, yes, because we have multiple copies of netError.xhtml. Filed bug 781688 and bug 781689, which I'll handle today. AFAICT, there are no more uses of it once those are fixed (I even checked comm-central!).

Comment 6

[Andrew McCreight [:mccr8]]

Great, thanks!

Comment 7

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to Justin Dolske [:Dolske] from comment #3)
> Why the capturing listener? So we trigger online-mode before the page starts
> the reload?

Yep.

> Probably worth a comment in retryThis that the browser.js code handles the
> go-online thing.

Will do.

Comment 8

[Blair McBride [:Unfocused] (UNAVAILABLE)]

https://hg.mozilla.org/integration/fx-team/rev/aa23416861c5

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/aa23416861c5

Comment 10

[[e-mail]]

(In reply to Blair McBride from comment #5)
> (I even checked comm-central!)
All you did was notice that we don't override aboutNetError.xhtml, but sadly you failed to inform us that we now need to create an alternative way of going online in front-end code...

Comment 11

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Sorry, yes, I meant to file a separate bug for that the other day, but forgot. Filed bug 782892.