network connection status api
Categories
(WebExtensions :: General, enhancement, P2)
Tracking
(firefox69 verified, firefox70 verified)
People
(Reporter: mixedpuppy, Assigned: mixedpuppy)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
(deleted),
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta+
|
Details |
(deleted),
application/x-xpinstall
|
Details |
Consider exposing some network connection events.
https://searchfox.org/mozilla-central/source/netwerk/base/nsINetworkLinkService.idl
we could have an api that has:
browser.network.getStatus() (up/down/etc)
browser.network.getLinkType() (wifi/usb/etc)
browser.network.onConnectionChanged
The onConnectionChanged would send an object essentially matching the values in nsINetworkLinkService whenever either "network:link-status-changed" or "network:link-type-changed" notifications are sent.
P3 for more info from dragana
Assignee | ||
Comment 1•5 years ago
|
||
Assignee | ||
Comment 2•5 years ago
|
||
Due to the simplicity of this api and that soft freeze has not happened yet, I wrote up the patch quickly. But IIUC :dragana has some concerns that the notifications could happen too much.
Comment 3•5 years ago
|
||
This should probably be dup'ed to bug 1514589. I didn't want to do that with the open patch here.
For reference, this could use some explanation about the use case, I could swear we had some discussion about exposing this earlier and decided not to do it, but I can't seem to find it.
Assignee | ||
Comment 4•5 years ago
|
||
(In reply to Andrew Swan [:aswan] from comment #3)
This should probably be dup'ed to bug 1514589.
Maybe, maybe not. That seems to be an external request for a public api, for now at least, I was intending this to be a privileged api to avoid public promises around it.
Assignee | ||
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Comment 6•5 years ago
|
||
Comment 7•5 years ago
|
||
There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:mixedpuppy, could you have a look please?
For more information, please visit auto_nag documentation.
Assignee | ||
Comment 8•5 years ago
|
||
This will wait for additional platform work targeting Fx70.
Assignee | ||
Comment 9•5 years ago
|
||
Since the network id landed, we'll try this for 69, better sooner than later for secure proxy.
Assignee | ||
Comment 10•5 years ago
|
||
Simple extension dumps status to console when network status updates.
Assignee | ||
Comment 11•5 years ago
|
||
While this will have a test, it uses a mock network service. The attached xpi will dump the status changes to the console. QE could use this to validate that they can see real network changes on the different platforms. e.g. turn wifi off then on. I see:
{"id":"bweB5kWjxX8iirGKroX5tWq38vg=","status":"down","type":"unknown"}
{"status":"down","type":"unknown"}
{"status":"up","type":"unknown"}
and on reload
{"id":"bweB5kWjxX8iirGKroX5tWq38vg=","status":"up","type":"unknown"}
There may be some timing issue about getting the network id via the onConnectionChanged handler.
Assignee | ||
Comment 12•5 years ago
|
||
another QE note: this is a privileged api, the attached extension can only be tested by loading via about:debugging on nightly or dev-ed.
Assignee | ||
Comment 13•5 years ago
|
||
Assignee | ||
Comment 14•5 years ago
|
||
Assignee | ||
Comment 15•5 years ago
|
||
Comment 16•5 years ago
|
||
Comment 17•5 years ago
|
||
bugherder |
Comment 18•5 years ago
|
||
Can this be uplifted to 69, in case we need it DoH?
Assignee | ||
Comment 19•5 years ago
|
||
(In reply to Nhi Nguyen (:nhi) from comment #18)
Can this be uplifted to 69, in case we need it DoH?
It's a larger chunk of code than I'd like to uplift, but I also don't feel it's a terribly risky uplift either. I'll put in a request, you may want to touch base with relman or whomever you'll need to convince if necessary.
Assignee | ||
Comment 20•5 years ago
|
||
Comment on attachment 9063919 [details]
Bug 1550605 add networkStatus api
Beta/Release Uplift Approval Request
- User impact if declined: requested by :nhi for secure-proxy.
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: Yes
- If yes, steps to reproduce: Attached extension will dump the network id to console, see comment 11
- List of other uplifts needed: None
- Risk to taking this patch: Medium
- Why is the change risky/not risky? (and alternatives if risky): This is a larger patch than I like uplifting, but there's still lots of time before next uplift, and I don't feel like anything in it is particularly scary. The dependency which contains the lower level platform support, bug 1561005, landed in time for 69.
- String changes made/needed: none
Assignee | ||
Updated•5 years ago
|
Updated•5 years ago
|
Comment 22•5 years ago
|
||
It would be good for QA to look at this first before we commit to uplifting. I'm not crazy about uplifting new APIs to Beta halfway through the cycle.
Assignee | ||
Comment 23•5 years ago
|
||
of note: values in this api are simply reflected from what platform provides us. The test I outlined for QE is providing them with a network id, but no network type. For myself, I currently get neither id or type. The API itself is working fine.
Comment 24•5 years ago
|
||
Verified as fixed in FF70 with the results described by Shane in comment #23.
Updated•5 years ago
|
Comment 25•5 years ago
|
||
Comment on attachment 9063919 [details]
Bug 1550605 add networkStatus api
Self-contained WebExtension API with test coverage. Approved for 69.0b10.
Comment 26•5 years ago
|
||
bugherder uplift |
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•3 years ago
|
Assignee | ||
Updated•2 years ago
|
Description
•