Last Comment Bug 779680 - about:neterror's GoOnline() should be invoked from events, not nsDOMWindowUtils
: about:neterror's GoOnline() should be invoked from events, not nsDOMWindowUtils
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Document Navigation (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla17
Assigned To: Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not)
:
Mentors:
Depends on:
Blocks: 782892 783763 790034
  Show dependency treegraph
 
Reported: 2012-08-01 16:31 PDT by Andrew McCreight [:mccr8]
Modified: 2012-09-10 12:52 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (4.26 KB, patch)
2012-08-02 21:27 PDT, Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not)
dolske: review+
bzbarsky: review+
Details | Diff | Review

Description Andrew McCreight [:mccr8] 2012-08-01 16:31:03 PDT
The current method is gross.
Comment 1 Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not) 2012-08-02 21:27:44 PDT
Created attachment 648605 [details] [diff] [review]
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 [:bz] (Out June 25-July 6) 2012-08-08 20:55:13 PDT
Comment on attachment 648605 [details] [diff] [review]
Patch v1

r=me
Comment 3 Justin Dolske [:Dolske] 2012-08-09 12:12:39 PDT
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] 2012-08-09 13:48:27 PDT
Is nsDOMWindowUtils::GoOnline still called anywhere after this patch?
Comment 5 Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not) 2012-08-09 16:28:14 PDT
(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] 2012-08-09 16:31:24 PDT
Great, thanks!
Comment 7 Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not) 2012-08-09 19:55:56 PDT
(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] (mostly unavailable, needinfo open, reviews not) 2012-08-09 20:46:02 PDT
https://hg.mozilla.org/integration/fx-team/rev/aa23416861c5
Comment 9 Tim Taubert [:ttaubert] 2012-08-13 21:21:10 PDT
https://hg.mozilla.org/mozilla-central/rev/aa23416861c5
Comment 10 neil@parkwaycc.co.uk 2012-08-15 01:18:00 PDT
(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] (mostly unavailable, needinfo open, reviews not) 2012-08-15 01:37:48 PDT
Sorry, yes, I meant to file a separate bug for that the other day, but forgot. Filed bug 782892.

Note You need to log in before you can comment on or make changes to this bug.