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] (UNAVAILABLE)
:
: Andrew Overholt [:overholt]
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] (UNAVAILABLE)
dolske: review+
bzbarsky: review+
Details | Diff | Splinter Review

Description User image Andrew McCreight [:mccr8] 2012-08-01 16:31:03 PDT
The current method is gross.
Comment 1 User image Blair McBride [:Unfocused] (UNAVAILABLE) 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 User image Boris Zbarsky [:bz] (still a bit busy) 2012-08-08 20:55:13 PDT
Comment on attachment 648605 [details] [diff] [review]
Patch v1

r=me
Comment 3 User image 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 User image Andrew McCreight [:mccr8] 2012-08-09 13:48:27 PDT
Is nsDOMWindowUtils::GoOnline still called anywhere after this patch?
Comment 5 User image Blair McBride [:Unfocused] (UNAVAILABLE) 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 User image Andrew McCreight [:mccr8] 2012-08-09 16:31:24 PDT
Great, thanks!
Comment 7 User image Blair McBride [:Unfocused] (UNAVAILABLE) 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 User image Blair McBride [:Unfocused] (UNAVAILABLE) 2012-08-09 20:46:02 PDT
https://hg.mozilla.org/integration/fx-team/rev/aa23416861c5
Comment 9 User image Tim Taubert [:ttaubert] 2012-08-13 21:21:10 PDT
https://hg.mozilla.org/mozilla-central/rev/aa23416861c5
Comment 10 User image 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 User image Blair McBride [:Unfocused] (UNAVAILABLE) 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.