The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla17

Status

()

Core
Document Navigation
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mccr8, Assigned: Unfocused)

Tracking

Trunk
mozilla17
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
The current method is gross.
Assignee: nobody → bmcbride
Status: NEW → ASSIGNED
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.)
Attachment #648605 - Flags: review?(dolske)
Attachment #648605 - Flags: review?(bzbarsky)
Comment on attachment 648605 [details] [diff] [review]
Patch v1

r=me
Attachment #648605 - Flags: review?(bzbarsky) → review+
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.
Attachment #648605 - Flags: review?(dolske) → review+
(Reporter)

Comment 4

5 years ago
Is nsDOMWindowUtils::GoOnline still called anywhere after this patch?
(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!).
(Reporter)

Comment 6

5 years ago
Great, thanks!
(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.
https://hg.mozilla.org/integration/fx-team/rev/aa23416861c5
https://hg.mozilla.org/mozilla-central/rev/aa23416861c5
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
(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...
Blocks: 782892
Sorry, yes, I meant to file a separate bug for that the other day, but forgot. Filed bug 782892.
Blocks: 783763
(Reporter)

Updated

5 years ago
Blocks: 790034
(Reporter)

Updated

5 years ago
No longer blocks: 775868
You need to log in before you can comment on or make changes to this bug.