Closed
Bug 782892
Opened 9 years ago
Closed 9 years ago
about:neterror no longer automatically disables Offline Mode, frontend should handle it
Categories
(SeaMonkey :: General, defect)
SeaMonkey
General
Tracking
(seamonkey2.14+ fixed, seamonkey2.15 fixed)
RESOLVED
FIXED
seamonkey2.15
People
(Reporter: Unfocused, Assigned: neil)
References
Details
Attachments
(2 files, 1 obsolete file)
6.11 KB,
patch
|
iann_bugzilla
:
review+
|
Details | Diff | Splinter Review |
1.26 KB,
patch
|
iann_bugzilla
:
review+
iann_bugzilla
:
approval-comm-aurora+
|
Details | Diff | Splinter Review |
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•9 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).
Reporter | ||
Comment 2•9 years ago
|
||
Bug 783763 will correct that log message, and move that test into /browser/
Depends on: 783763
![]() |
||
Updated•9 years ago
|
Assignee: nobody → philip.chee
Status: NEW → ASSIGNED
Version: unspecified → Trunk
![]() |
||
Comment 3•9 years ago
|
||
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•9 years ago
|
||
This version seems to pass your test, as far as I can tell...
Attachment #656243 -
Flags: feedback?(philip.chee)
![]() |
||
Comment 5•9 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•9 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•9 years ago
|
||
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•9 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 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•9 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•9 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•9 years ago
|
||
Pushed: http://hg.mozilla.org/comm-central/rev/0e501d59baac http://hg.mozilla.org/comm-central/rev/b7d36ee71002 (test)
![]() |
||
Comment 13•9 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•9 years ago
|
tracking-seamonkey2.14:
--- → +
Comment 14•9 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•9 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
Closed: 9 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.
Description
•