The default bug view has changed. See this FAQ.

about:neterror no longer automatically disables Offline Mode, frontend should handle it

RESOLVED FIXED in seamonkey2.15

Status

SeaMonkey
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Unfocused, Assigned: neil@parkwaycc.co.uk)

Tracking

Trunk
seamonkey2.15
Dependency tree / graph

SeaMonkey Tracking Flags

(seamonkey2.14+ fixed, seamonkey2.15 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Usage of nsIDOMWindowUtils.goOnline() was removed from about:neterror in bug 779680, as that method is going away (bug 775868). Previously this was used to automatically disable Offline Mode when clicking "retry", after a page couldn't be loaded due to being in Offline Mode.

Bug 779680 handled this in Firefox by having the browser chrome listen for a click event on the errorTryAgain button in about:neterror (via a capturing event listener), checking "e=netOffline" is in the URL, and doing:
  Services.io.offline = false;

Comment 1

5 years ago
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1345241103.1345244340.1392.gz#err7

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/docshell/test/browser/browser_bug435325.js | After clicking the Try Again button, we're back  online. This depends on Components.interfaces.nsIDOMWindowUtils being available from untrusted content (bug 435325).
Bug 783763 will correct that log message, and move that test into /browser/
Depends on: 783763

Updated

5 years ago
Assignee: nobody → philip.chee
Status: NEW → ASSIGNED
Version: unspecified → Trunk

Comment 3

5 years ago
Created attachment 656070 [details] [diff] [review]
Patch v1.0 Use TabsProgressListener

This patch works when I test off-line manually but the test copied from Firefox fails for a reason I cannot discern.
Attachment #656070 - Flags: feedback?(neil)
(Assignee)

Comment 4

5 years ago
Created attachment 656243 [details] [diff] [review]
Possible patch

This version seems to pass your test, as far as I can tell...
Attachment #656243 - Flags: feedback?(philip.chee)

Comment 5

5 years ago
Comment on attachment 656243 [details] [diff] [review]
Possible patch

Passes all the tests.

*** Start BrowserChrome Test Results ***
TEST-INFO | checking window state
TEST-START | chrome://mochitests/content/browser/suite/browser/test/browser_bug435325.js
TEST-INFO | chrome://mochitests/content/browser/suite/browser/test/browser_bug435325.js | got about:blank, which is expected once, so return
Error loading URL http://example.com/ : 804b0046 (NS_ERROR_DOCUMENT_NOT_CACHED)
TEST-PASS | chrome://mochitests/content/browser/suite/browser/test/browser_bug435325.js | Setting Services.io.offline to true.
TEST-PASS | chrome://mochitests/content/browser/suite/browser/test/browser_bug435325.js | Loading the Offline mode neterror page.
TEST-PASS | chrome://mochitests/content/browser/suite/browser/test/browser_bug435325.js | The errorpage has got a #errorTryAgain element
TEST-PASS | chrome://mochitests/content/browser/suite/browser/test/browser_bug435325.js | After clicking the Try Again button, we're back online.
INFO TEST-END | chrome://mochitests/content/browser/suite/browser/test/browser_bug435325.js | finished in 187ms
TEST-INFO | checking window state

INFO TEST-START | Shutdown
Browser Chrome Test Summary
        Passed: 4
        Failed: 0
        Todo: 0

*** End BrowserChrome Test Results ***

> +        <![CDATA[
> +          if (/^about:neterror\?e=netOffline/.test(event.target.documentURI))
> +            event.target.addEventListener("click", function(event) {
Please name this anonymous function.

[As discussed over IRC, while it may be best practice to remove the eventListener afterwards, it may not be practicable to do so.]

> +              if (event.target.id == "errorTryAgain")
> +                Services.io.offline = false;
This adds a new dependency on Services.
Your bonus for 10: ioservice is used in several other places. Consider turning it into a field.

> +            }, true);
> +        ]]>

f=me
Attachment #656243 - Flags: feedback?(philip.chee) → feedback+
(Assignee)

Comment 6

5 years ago
(In reply to Philip Chee from comment #5)
> Your bonus for 10: ioservice is used in several other places. Consider
> turning it into a field.
It's used inside a callback, so I'd have to mess around with bind or some such...
(Assignee)

Comment 7

5 years ago
Created attachment 656612 [details] [diff] [review]
Proposed patch
Assignee: philip.chee → neil
Attachment #656070 - Attachment is obsolete: true
Attachment #656243 - Attachment is obsolete: true
Attachment #656070 - Flags: feedback?(neil)
Attachment #656612 - Flags: review?(iann_bugzilla)

Comment 8

5 years ago
> It's used inside a callback, so I'd have to mess around with bind or some such...
Yeah, too much effort for not enough gain.

Comment 9

5 years ago
Comment on attachment 656612 [details] [diff] [review]
Proposed patch

Don't we still need the test from the first patch?
Attachment #656612 - Flags: review?(iann_bugzilla) → review+

Comment 10

5 years ago
> Don't we still need the test from the first patch?
Yes. I can check both in if you r+ the test.

Comment 11

5 years ago
Comment on attachment 656070 [details] [diff] [review]
Patch v1.0 Use TabsProgressListener

r=me for the test
Attachment #656070 - Attachment is obsolete: false
Attachment #656070 - Flags: review+

Comment 12

5 years ago
Pushed:
http://hg.mozilla.org/comm-central/rev/0e501d59baac
http://hg.mozilla.org/comm-central/rev/b7d36ee71002 (test)

Comment 13

5 years ago
Comment on attachment 656612 [details] [diff] [review]
Proposed patch

[Approval Request Comment]
Regression caused by (bug #): 779680
User impact if declined: Unable to go online when the "Try Again" button is pressed.
Testing completed (on m-c, etc.): comm-central.
Risk to taking this patch (and alternatives if risky): minimal risk, just missed the train. Bug 779680 landed on mozilla17 so we need to land this on comm-aurora.
String changes made by this patch: none.
Attachment #656612 - Flags: approval-comm-aurora?

Updated

5 years ago
tracking-seamonkey2.14: --- → +

Comment 14

5 years ago
Comment on attachment 656612 [details] [diff] [review]
Proposed patch

a=me for this (and test if required)
Attachment #656612 - Flags: approval-comm-aurora? → approval-comm-aurora+

Comment 15

5 years ago
Pushed to comm-aurora:
http://hg.mozilla.org/releases/comm-aurora/rev/ada7d03ccea4
http://hg.mozilla.org/releases/comm-aurora/rev/ee74d240d1ce (test)
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
status-seamonkey2.14: --- → fixed
status-seamonkey2.15: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.15
You need to log in before you can comment on or make changes to this bug.