1354721 - about:geo page for network geolocation status reporting

Status: RESOLVED WONTFIX

(Core :: DOM: Geolocation, enhancement)

Description

[:garvan]

I have a patch to add about:geo as an unlisted about page (need to type about:geo directly to get to it). 
We can tell users who report bugs to use this page and paste the log in.
It has a button to start/stop the network geolocation provider, and a text area that for provider debug logging. 

Useful stuff in the logs:
- number of wifis seen, the number sent in the network request
- the server response
- the state of the local cache (consecutive network requests are only sent if certain conditions are met)

A sample log would be:

startup called.
Sending request
Scanned WIFI AP count: 8, Sending WIFI AP count: 8, Cell count: 0
server returned status: 200 --> {"location":{"lat":43.6604989,"lng":-79.4389353},"accuracy":13.7301629}
43.6604989, -79.4389353
Use request cache:true reason:New req. is GeoIP.
43.6604989, -79.4389353
Use request cache:true reason:EqualCells:false EqualWifis:true, Wifi only.
43.6604989, -79.4389353

Comment 1

[:garvan]

Attachment: aboutGeo.diff

TODO: try server it, and find reviewer

Comment 2

[:garvan]

Attachment: screenshot of page in action

Comment 3

[:garvan]

Attachment: custom permission prompt

Not sure if I can access the built-in geo prompt without hackery, no matter, just show a custom one.

Comment 4

[:garvan]

Attachment: Bug 1354721 - Add to XPIDL nsIGeo.Prov.: debugLogger() and

listener function wrapper and the call to non-existent
 listener.locationUpdatePending()

Review commit: https://reviewboard.mozilla.org/r/128926/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/128926/

Comment 5

[:garvan]

Attachment: Bug 1354721 - add about:geo xhtml page that can show network

geo logging

Review commit: https://reviewboard.mozilla.org/r/128928/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/128928/

Comment 6

[:garvan]

Comment on attachment 8857033 [details]
Bug 1354721 - Add to XPIDL nsIGeo.Prov.: debugLogger() and

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/128926/diff/1-2/

Comment 7

[:garvan]

Attachment: Bug 1354721 - Cleanup network geo provider: removed unneeded

listener function wrapper and the call to non-existent
 listener.locationUpdatePending()

Review commit: https://reviewboard.mozilla.org/r/128930/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/128930/

Comment 8

[:garvan]

Hi Michelangelo, I am looking for a reviewer on this, interested?

Comment 9

[Michelangelo De Simone [Offline] [:mds]]

(In reply to :garvan from comment #8)

> Hi Michelangelo, I am looking for a reviewer on this, interested?

Hey, sorry for the delay.

It's a pretty neat idea: I'm just not sure whether and how we want to expose such data to the end user.
Probably - and I'm speculating here - we should ping somebody from UX?
Let's check with :jdm and see what his thoughts are.

Comment 10

[:garvan]

Agreed it does have UX implications. Moreover, I ignored localization of this.

Other debugging information about pages are similarly primitive –but the page styling I have here is clearly wrong (now that I look closer at other about pages), so that should be fixed up at a minimum.

This patch probably needs an 'about' pages owner approval once I get past the geo-devs review.

Maybe it could be nightly only, and for bugs where users are reporting geo problems we can tell them to grab nightly and send us the log, or check it themselves. 

Triage for these "Fx is not geolocating me" bugs I see as:
- is >2 WIFI APs available? (if so, typically MLS or GLS DB doesn't have data for user's region)
- is GeoIP? (if so, typical problem is with GeoIP db for user's region)
- if WIFI not scanning, then problem could be in the WIFI scanning stack (either Fx or OS), investigate further
- we could also be logging which provider is being used (in case user changed default provider, or maybe there is some bug where wrong provider is used). IIRC there was a fallback provider system on Windows, but I have been away for too long and either forgot or willing chose to not recall the details there.

Any one of these could be 'investigate further' items of course. I think in most cases geo is working 'as expected' (which is not always 'as user expected' or saw in another application which is using a different geolocation stack).

Comment 11

[:garvan]

Comment on attachment 8857039 [details]
Bug 1354721 - add about:geo xhtml page that can show network

https://reviewboard.mozilla.org/r/128928/#review132716

::: browser/base/content/aboutGeo.xhtml:60
(Diff revision 1)
> +      let notify  = new PopupNotifications(browser,
> +                    domWindow.document.getElementById("notification-popup"),
> +                    domWindow.document.getElementById("notification-popup-box"));
> +
> +      let hasPermission = false;
> +      let isRunning = false;

not needed, if geo non-null, then geo is running

::: browser/base/content/aboutGeo.xhtml:74
(Diff revision 1)
> +          geo.enableLoggingToListener(true);
> +          geo.startup();
> +      }
> +
> +      function startStopToggled() {
> +        let button = document.getElementById("startStopButton");

move next to usage on line 77

::: browser/base/content/aboutGeo.xhtml:91
(Diff revision 1)
> +          return;
> +        }
> +
> +        notify.show(browser.selectedBrowser, "geopopup",
> +          "Allow location access for testing?", null,
> +          { label: "Don't Allow", accessKey: "a", callback: () => {} },

accessKey seemed to be required, put proper key in for this (is it first letter of label maybe?)

::: browser/base/content/aboutGeo.xhtml:101
(Diff revision 1)
> +            }
> +          }]
> +        );
> +      }
> +
> +      window.onclose = () => {

should change to beforeclose event, I don't think I have ever had onclose work reliably

Comment 12

[Hanno Schlichting]

(In reply to :garvan from comment #10)
> Maybe it could be nightly only, and for bugs where users are reporting geo
> problems we can tell them to grab nightly and send us the log, or check it
> themselves. 

I think if we add this page, it should be on all channels. Especially as the Geo stack and providers we use is different on the different channels. We also ran into problems where we deployed the wrong GEO API keys to some of the built machines (so only Windows on all channels was broken). We also have some users on builds not produced by us (for example Linux package managers), and in those cases the reason why something is broken might again be different.

Given recent bugs, we also typically only get bug reports when problems made it into the release channel, so that's where we'd need the debug information the most.

> Triage for these "Fx is not geolocating me" bugs I see as:
> - is >2 WIFI APs available? (if so, typically MLS or GLS DB doesn't have
> data for user's region)
> - is GeoIP? (if so, typical problem is with GeoIP db for user's region)
> - if WIFI not scanning, then problem could be in the WIFI scanning stack
> (either Fx or OS), investigate further
> - we could also be logging which provider is being used (in case user
> changed default provider, or maybe there is some bug where wrong provider is
> used). IIRC there was a fallback provider system on Windows, but I have been
> away for too long and either forgot or willing chose to not recall the
> details there.

We are in the process of changing the default Geo providers on non-release channels. So by the time 55 is in beta, beta/aurora/nightly will all use MLS + Core Location (with MLS fallback) and Windows Location (with MLS fallback). There's also a separate compile time option where someone can built Firefox with additional libgps support on Linux, but that's rarely used. The code for the Gonk GPS provider is also still there, but not used by anyone anymore.

Release and late-beta (the second 3 weeks of the beta period) will stay as-is.

Comment 13

[:garvan]

Yeah, having on all channels makes the most sense as the goal is to minimize the barrier to end users to report problems.


> <snip> So by the time 55 is in beta, beta/aurora/nightly will all use MLS + ...
I only logged debug info for MLS provider -the CoreLocation and Windows provider both have an empty debug log function implementation. I'll change that to log some rudimentary debug info in those before landing.

Comment 14

[Josh Matthews [:jdm]]

I'm also in favour of this. I doubt that this needs any sign-off form UX unless we're exposing ways to access it beyond typing the about:geo into the location bar.

Comment 15

[:garvan]

Thanks Josh, I'll add the final logging bits that Hanno mentioned and get this patch in-shape for review.

Comment 16

[:garvan]

I am closing this old bug as won't fix, but someone is welcome to re-open.
Reasons:
- Time has shown there aren't enough cases where this is needed. This debug info can be logged to the console, and more work would be needed to provide info that isn't easily available in the console output.
- more code in Firefox increases maintenance burden, and need for this PR is too low to increase that.
- the code in this bug is a useful starting point if it is needed