Closed Bug 1653549 Opened 2 years ago Closed 6 months ago

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

Categories

(Core :: DOM: Geolocation, defect)

78 Branch
defect

Tracking

()

RESOLVED FIXED
92 Branch
Tracking Status
firefox92 --- fixed

People

(Reporter: konrad, Assigned: marcos)

Details

(Keywords: parity-chrome, parity-safari)

Attachments

(1 file)

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

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

Component: Untriaged → DOM: Core & HTML
Product: Firefox → Core
Severity: -- → S3
Priority: -- → P3
Component: DOM: Core & HTML → DOM: Geolocation

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

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

Priority: P3 → --

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

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

Flags: needinfo?(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!

Flags: needinfo?(jstutte)
Flags: needinfo?(echen)

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.

(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...

Flags: needinfo?(echen)

(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.

Assignee: nobody → marcos
Pushed by marcos@marcosc.com:
https://hg.mozilla.org/integration/autoland/rev/a6fc75e24e36
don't sent a location if document is hidden r=edgar
Flags: needinfo?(marcos)

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

Flags: needinfo?(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?

Flags: needinfo?(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.

Flags: needinfo?(smolnar)

Thanks Sandor, that's exactly what I needed!

Pushed by marcos@marcosc.com:
https://hg.mozilla.org/integration/autoland/rev/5388c654aee1
don't sent a location if document is hidden r=edgar
Status: UNCONFIRMED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → 92 Branch
You need to log in before you can comment on or make changes to this bug.