1653549 - Site with geolocation API access granted can get location updates while in the background

Status: RESOLVED FIXED

(Core :: DOM: Geolocation, defect)

Description

[Konrad Dzwinel]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_6) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/13.1.2 Safari/605.1.15
Firefox for Android

Steps to reproduce:

1. Go to https://output.jsbin.com/yapihuf
2. Allow geolocation API access to the site
3. Switch to a different tab, or app, for 5s
4. Come back to the tab from #1

Actual results:

Site was receiving updates while in the background (page will show multiple new entries with a "Last update 1s ago" message).

Expected results:

Site should not receive updates when its not focused (page should only show one new entry with a "Last update 5s ago" message).

This affects Firefox on desktop and Android. It does not affect Chrome or Safari.

It's not required by the Geolocation API specification ATM, but it's a privacy concern and it's being discussed here: https://github.com/w3c/geolocation-api/issues/48

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Marcos Caceres [:marcosc]]

We will need to spec this out as part of https://github.com/w3c/geolocation-api/issues/48

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The component has been changed since the backlog priority was decided, so we're resetting it.
For more information, please visit auto_nag documentation.

Comment 4

[marcos]

Attachment: Bug 1653549 - don't sent a location if document is hidden

when the document is hidden from view, silently drop position updates on the floor. This aligns with Chrome and Safari.

Comment 5

[marcos]

Hi Jens, not sure who to ping about this bug for guidance (i.e., not sure who owns geo now). I wouldn't mind some initial feedback to make sure I'm on the right track before adding tests?

FWIW, the test site works as expected when the patch is applies:
https://output.jsbin.com/yapihuf

Comment 6

[Jens Stutte [:jstutte]]

(In reply to marcos from comment #5)

Hi Jens, not sure who to ping about this bug for guidance (i.e., not sure who owns geo now). I wouldn't mind some initial feedback to make sure I'm on the right track before adding tests?

Well, I am the triage owner, so you chose the right entry point. The change looks reasonable to me and is something we should probably do. For further review please refer to :echen. Thanks for your help!

Comment 7

[marcos]

Hi Edgar, I've done some more exploring and this is insufficient. I think I need to set up visibility change event listeners if the document is hidden, then kick off the update request after the document becomes visible again.

Comment 8

[Edgar Chen [:edgar]]

(In reply to marcos from comment #7)

Hi Edgar, I've done some more exploring and this is insufficient. I think I need to set up visibility change event listeners if the document is hidden, then kick off the update request after the document becomes visible again.

Yeah, for watchPosition(), I think it makes sense to kick off an update request once the document becomes visible.

But for getCurrentPosition(), Chrome seems just ignore the request, i.e. no PositionCallBack or PositionErrorCallBack get called even after document becomes visible? Chrome's behavior is also odd, I think we should at least call PositionErrorCallBack if PositionCallBack isn't triggered. But I guess doing it might be a compatibility risk...

Comment 9

[marcos]

(In reply to Edgar Chen [:edgar] from comment #8)

(In reply to marcos from comment #7)

Hi Edgar, I've done some more exploring and this is insufficient. I think I need to set up visibility change event listeners if the document is hidden, then kick off the update request after the document becomes visible again.

Yeah, for watchPosition(), I think it makes sense to kick off an update request once the document becomes visible.

But for getCurrentPosition(), Chrome seems just ignore the request, i.e. no PositionCallBack or PositionErrorCallBack get called even after document becomes visible? Chrome's behavior is also odd, I think we should at least call PositionErrorCallBack if PositionCallBack isn't triggered. But I guess doing it might be a compatibility risk...

I did some thinking and I think what I had originally might work... basically, we can just drop the position updates on the floor until the document becomes visible/fully active... I'll write some tests to confirm it works.

Comment 10

[marcos]

Try:
https://treeherder.mozilla.org/jobs?repo=try&revision=02ad559f16e973c8cc37f9b518029548a6bc2689

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a6fc75e24e36
don't sent a location if document is hidden r=edgar

Comment 12

[Sandor Molnar[:smolnar]]

Backed out for causing mochitest failures in test_hidden

Backout link: https://hg.mozilla.org/integration/autoland/rev/e69cfd0df6fc246c478ee575d7f63bed26e63848

Push with failures

Failure log

Comment 13

[marcos]

Ah yeah, the test will fail on Android... that makes sense. We are doing tab switching.

Comment 14

[marcos]

Try Android and few others explicitly:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c41e071c5019ff87ca0448d015c23e98b5929ae4

Try auto/fuzzy:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8963ad40b358c3b0f13ee58c2f7dc739aab81380

Comment 15

[marcos]

Hi Sandor, just wondering, I normally do ./mach try before merging a patch, but it fails to catch things like that above.

I've now used try chooser to make sure that Android is tested.

However, is there something else I should be doing in the first place so that ./mach try checks across all OSs?

Comment 16

[Sandor Molnar[:smolnar]]

Hi, another alternative you could try is using the Fuzzy selector ./mach try fuzzy for choosing jobs
From what I can see on your try push, only the android builds have been triggered.

Comment 17

[marcos]

Thanks Sandor, that's exactly what I needed!

Comment 18

[marcos]

Try (Android/Mochitests):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4ce95737e3397b1b3a298a4e727b033f6666b443

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5388c654aee1
don't sent a location if document is hidden r=edgar

Comment 20

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/5388c654aee1